-
-
Notifications
You must be signed in to change notification settings - Fork 355
fix(core/plugin): Ensure android plugin type safety #5098
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
fix(core/plugin): Ensure android plugin type safety #5098
Conversation
| const cfg = withAppBuildGradle(config, config => { | ||
| if (config.modResults.language === 'groovy') { | ||
| config.modResults.contents = modifyAppBuildGradle(config.modResults.contents); | ||
| const cfg = withAppBuildGradle(config, appBuildGradle => { |
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.
nice catch!
|
Overall looks good! I will do final checks tomorrow and if all good, I will approve it, |
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.
Thank your contribution @justnero 🙇
The changes LGTM!
Leaving the final approval/merge to @lucas-zimerman in case he has more feedback 🙏
lucas-zimerman
left a comment
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.
LGTM!
📢 Type of change
📜 Description
packages/core/plugin/src/withSentryAndroidGradlePlugin.ts💡 Motivation and Context
Fixes #5097
💚 How did you test it?
And a manual change test on my project to verify it's actually the only reason.
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
This probably misses the same
expect.toEqualassertions for other branches, but overall is guaranteed by the type safety itself.