-
-
Notifications
You must be signed in to change notification settings - Fork 221
ignore Android binding warnings #4423
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
Conversation
…warnings - Enable TreatWarningsAsErrors for Android bindings project - Suppress expected/unavoidable Android binding warnings (BG8xxx codes) - These warnings are caused by Java interface circular dependencies in Sentry SDK - Warnings are documented with explanations for each type - New warning types will now break the build for review - Analyzed 1528 warnings and confirmed they're due to binding limitations - Solution preserves existing Metadata.xml strategic type removals
<!-- BG8604: Missing ancestor types - nested types whose parents are removed --> | ||
<!-- BG8502: Interface invalidation - cascade effect from other binding issues --> | ||
<!-- BG8401/BG8400: Field/property conflicts - resolved individually in Metadata.xml where needed --> | ||
<NoWarn>$(NoWarn);BG8801;BG8C01;BG8701;BG8800;BG8700;BG8605;BG8606;BG8604;BG8502;BG8401;BG8400;BG8503;BG8C00</NoWarn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: also mention BG8503
and BG8C00
in the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should I add as a comment?
Unless there's a plan to fix them, I just want all warnings gone. Should we leave the warnings on the terminal?
Doesn't seem like any of this is worth doing anything about since we're not binding the whole library and it works as inteded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about a short description of these warning for completeness.
I added (non-blocking) suggestions.
<!-- BG8801: Invalid parameter types - caused by circular interface dependencies in Sentry Java SDK --> | ||
<!-- BG8C01: Invalid base interface - interfaces reference types removed by Metadata.xml --> | ||
<!-- BG8701: Invalid return types - return types reference unresolvable interfaces --> | ||
<!-- BG8800/BG8700: Unknown/Invalid types - types intentionally removed by Metadata.xml --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<!-- BG8800/BG8700: Unknown/Invalid types - types intentionally removed by Metadata.xml --> | |
<!-- BG8800/BG8700: Unknown parameter/return types - types intentionally removed by Metadata.xml --> |
Invalid parameter/return types are BG8801
/BG8701
.
Co-authored-by: Stefan Pölz <[email protected]>
Co-authored-by: Stefan Pölz <[email protected]>
Building now creates lots of warnings.
Even though the solution had TreatWarningAsErrors, the build passes, which means these warnings are not 'normal' and sneak in and grow over time.
I'm ignoring all warnings that we can't take action (trusting Cursor pretty much on those claims).
But after a few iterations, I couldn't get these types of warnings to break the build, so if new ones are introduced, it'll take someone paying attention and ignoring them explicitly.
Before:
Cursors claims:
Turns out there's not much we can do about these, so we'll ignore them
Enable TreatWarningsAsErrors for Android bindings projectrevertedNew warning types will now break the build for reviewnot true#skip-changelog