Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Trim warnings in DiagnosticSource and WinUIUnhandledException integrations #3410

Merged
merged 7 commits into from
Jun 7, 2024

Conversation

jamescrosswell
Copy link
Collaborator

@jamescrosswell jamescrosswell commented Jun 6, 2024

Resolves #3304

TODO: Testing

  • We should add an integration test (maybe using pester) to ensure we don't get any trimming warnings when publishing the Sentry.Samples.Console.Basic project with trimming enabled
  • Ideally we'd have some tests to ensure the WinUI and DiagnosticSource integrations would not be registered if trimming is enabled as well... we could maybe make that easier if we added an internal setter to AotHelper.IsTrimmed so that we could "mock" this during testing.

@jamescrosswell jamescrosswell changed the title Fixed Trim warnings in Sentry.DiagnosticSource Fixed Trim warnings in DiagnosticSource and WinUIUnhandledException integrations Jun 6, 2024
/// </remarks>
internal static class ReflectionHelper
{
[UnconditionalSuppressMessage("TrimAnalyzer", "IL2075", Justification = AotHelper.SuppressionJustification)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: Could we have added the attribute where this was moved from, instead of having that in a new ReflectionHelper?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have yes. The main reason I moved it was to put it in the DiagnosticSource namespace (to make it clear that it wasn't intended to be used beyond that scope - it's a bit of a retro way of instrumenting things and I don't think we want to replicate this anywhere else).

It was in a MiscExtensions class so it it wasn't closely related to any of the other functionality that we pulled this away from.

Copy link
Contributor

@bitsandfoxes bitsandfoxes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bitsandfoxes bitsandfoxes merged commit 9f36d22 into main Jun 7, 2024
20 checks passed
@bitsandfoxes bitsandfoxes deleted the trim-warnings branch June 7, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sentry assembly produces trim warnings when compiling AOT
2 participants