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

Add support to Gradle 8+ #1427

Merged
merged 5 commits into from May 10, 2023
Merged

Add support to Gradle 8+ #1427

merged 5 commits into from May 10, 2023

Conversation

davidmartos96
Copy link
Contributor

📜 Description

Hello, currently the plugin doesn't build when Gradle 8+ is used. That is because the property namespace is mandatory.

  • I updated the Android example project.
  • I included the namespace property with backwards compatibility

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

@krystofwoldrich
Copy link
Member

@marandaneto @davidmartos96 Maybe an integration test to check that the plugin builds with Gradle 7 and 8 would make sense to add?

@davidmartos96
Copy link
Contributor Author

@krystofwoldrich Is there any test of that kind in the repo? I'm not sure how it would work or if it would be worth the effort.

This change is equivalent to the one the official plugins have recently pushed in latest versions.
Like here: https://pub.dev/packages/shared_preferences_android/changelog
Here is the commit: flutter/packages@a86beaf
The if statement is for Gradle versions not compatible with the namespace attribute. It should build fine for different Gradle versions.

1 similar comment
@davidmartos96
Copy link
Contributor Author

@krystofwoldrich Is there any test of that kind in the repo? I'm not sure how it would work or if it would be worth the effort.

This change is equivalent to the one the official plugins have recently pushed in latest versions.
Like here: https://pub.dev/packages/shared_preferences_android/changelog
Here is the commit: flutter/packages@a86beaf
The if statement is for Gradle versions not compatible with the namespace attribute. It should build fine for different Gradle versions.

@marandaneto
Copy link
Contributor

@marandaneto @davidmartos96 Maybe an integration test to check that the plugin builds with Gradle 7 and 8 would make sense to add?

Since we don't use AGP 8 yet, this is an integration test per see :)
Also every flutter package does this already flutter/packages@a86beaf
Its safe to go, otherwise we'd need to set up another sample app just for that (CI overhead, etc), but I'd do if the change was more fragile.

@marandaneto
Copy link
Contributor

Thanks @davidmartos96

@marandaneto
Copy link
Contributor

@davidmartos96
Copy link
Contributor Author

@marandaneto Done!

@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (0006698) 90.23% compared to head (08a79dc) 90.23%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1427   +/-   ##
=======================================
  Coverage   90.23%   90.23%           
=======================================
  Files         181      181           
  Lines        5788     5788           
=======================================
  Hits         5223     5223           
  Misses        565      565           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marandaneto marandaneto enabled auto-merge (squash) May 10, 2023 07:20
@marandaneto marandaneto merged commit 3a43905 into getsentry:main May 10, 2023
85 of 86 checks passed
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.

None yet

3 participants