-
Notifications
You must be signed in to change notification settings - Fork 50
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
Migrate to Gradle version catalog #1059
Migrate to Gradle version catalog #1059
Conversation
1b6eb17
to
e26b5cd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1059 +/- ##
=======================================
Coverage 85.76% 85.76%
=======================================
Files 176 176
Lines 6222 6222
Branches 853 853
=======================================
Hits 5336 5336
Misses 551 551
Partials 335 335 ☔ View full report in Codecov by Sentry. |
build.gradle
Outdated
alias libs.plugins.androidx.navigation.safeargs apply false | ||
// TODO: is this needed? | ||
alias libs.plugins.kotlin.kapt apply false | ||
// TODO: is this needed? |
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.
Hmm are these TODO
's meant to be removed?
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.
Looks like we only use parcelization in the public
and feature/amazon
modules, so I believe it should only be needed in those...
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.
Looks good but we should decide what to do with those todos
build.gradle
Outdated
alias libs.plugins.androidx.navigation.safeargs apply false | ||
// TODO: is this needed? | ||
alias libs.plugins.kotlin.kapt apply false | ||
// TODO: is this needed? |
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.
Looks like we only use parcelization in the public
and feature/amazon
modules, so I believe it should only be needed in those...
@tonidero removed those todos. I couldn't figure out how to remove parcelize from the root build.gradle, though I also added a test bundle and extracted more versions |
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.
Left a few more comments but it looks good!
id "com.savvasdalkitsis.module-dependency-graph" version "0.9" | ||
alias libs.plugins.mavenPublish apply false | ||
alias libs.plugins.android.application apply false | ||
alias libs.plugins.android.library apply false |
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.
So now we are adding both the application and library plugins here and applying them on the respective modules each. Is it necessary to specify both here?
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.
Yes. Think this is the equivalent of adding the AGP plugin. Removing either plugin gives the same error as removing the parcelize (The request for this plugin could not be satisfied because the plugin is already on the classpath with an unknown version, so compatibility cannot be checked.). Removing application
gives the issue in the apps, I am getting it in api-tester
, and removing the library one gives it in common
.
I think these plugins need to be applied both in the base and in the module's build.gradle. I guess it makes sense for the android plugins, because at the end of the day the need to build an android project, and this is the build.gradle of the project, but I am still not sure about the parcelize one.
**This is an automatic release.** ### Bugfixes * Default customer info schema version to latest known by SDK (#1080) via Toni Rico (@tonidero) * Handle other diagnostics-related exceptions (#1076) via Toni Rico (@tonidero) * Return error in queryPurchases if error connecting to billing client (#1072) via Toni Rico (@tonidero) ### Other Changes * Fix offline entitlements integration tests (#1085) via Toni Rico (@tonidero) * Add defaultsRelease variant tests run configuration (#1074) via Toni Rico (@tonidero) * Compose sample app: move to gradle catalog (#1081) via Toni Rico (@tonidero) * Compose sample app: automate builds (#1082) via Toni Rico (@tonidero) * Compose sample app (#1056) via Toni Rico (@tonidero) * Migrate to Gradle version catalog (#1059) via Cesar de la Vega (@vegaro) * Trusted entitlements: Add logs with verification mode (#1067) via Toni Rico (@tonidero) * Sync pending purchases before getting customer info (#1073) via Toni Rico (@tonidero) * Refactor syncing pending transactions logic out of `Purchases` (#1058) via Toni Rico (@tonidero) * Refactor CustomerInfo listener and cache logic into CustomerInfoUpdater (#1052) via Toni Rico (@tonidero) * Trusted entitlements: Add integration tests (#1071) via Toni Rico (@tonidero) * Trusted entitlements: Add internal mechanism to force signing errors for tests (#1070) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <[email protected]>
Hey @vegaro did I get listed as a contributor to this? 🙏 |
@mikescamell You actually didn't, but you should have, looks like a bug both in GitHub and in our changelog generation script. It's weird because you appear in https://github.com/RevenueCat/purchases-android/graphs/contributors but when clicking in the Your name also appears in the commit in main so I am not sure why GitHub is not picking it up: I also noticed you appear in the 6.5.0 release https://github.com/RevenueCat/purchases-android/releases/tag/6.5.0, so not sure why this change didn't pick up your name I will fix the changelog so your name is part of it. I will also give it a thought to see how we can change the external contributions flow so this doesn't happen again Sorry about that! And thanks again for your contribution and for bringing it up! |
I kept expecting to get an email with a tag from one of the releases and that's when I noticed the merge commit didn't mention me and I wondered what happened, seeing as I had no issues from my previous contribution as you said. I felt a little weird asking to be honest 😅 Thanks for making the effort to add me in 🙌 |
Merging #1033 into main