-
Notifications
You must be signed in to change notification settings - Fork 64
Introduce Metalava for tracking the public API surfaces #2370
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
Generated by 🚫 Danger |
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.
TIL about Metalava! I think this is great! Just some suggestions.
purchases/api.txt
Outdated
@com.revenuecat.purchases.InternalRevenueCatAPI @kotlin.jvm.JvmInline @kotlinx.serialization.Serializable public final value class ColorAlias { | ||
ctor public ColorAlias(String value); | ||
} |
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.
Would it be possible to ignore anything annotated with @InternalRevenueCatAPI
?
In the Metalava docs I read:
hide code intended to be implementation only, driven by javadoc comments like
@hide
,@doconly
,@removed
, etc, as well as various annotations.
But maybe we can somehow automate that, so @InternalRevenueCatAPI
is automatically ignored?
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.
That makes sense. I just hid all the public classes and methods that are marked with @InternalRevenueCatAPI
.
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, I love that it's just hiddenAnnotations.add("com.revenuecat.purchases.InternalRevenueCatAPI")
!
@@ -12,6 +12,7 @@ plugins { | |||
alias(libs.plugins.kover) apply false | |||
alias(libs.plugins.emerge) apply false | |||
alias(libs.plugins.poko) apply false | |||
alias(libs.plugins.metalava) 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.
Could you try adding this to our CircleCI config? We could have a new validate-binary-compatibility
job that does something like:
- run:
name: Validate binary compatibility (Defaults)
command: ./gradlew metalavaCheckCompatibilityDefaultsRelease
- run:
name: Validate binary compatibility (CustomEntitlementComputation)
command: ./gradlew metalavaCheckCompatibilityCustomEntitlementComputationRelease
This can be added to the existing build-test-deploy
workflow. Let me know if you have any questions!
(This comment also has nothing to do with this line.)
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 just added a CI configuration for checking metalava here: 220c979, and would love to get your double-check 😄
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 believe you just need to add the matalava
job to the build-test-deploy
workflow at the end of that file and it should be good! It should appear as one of the jobs running in this PR when configured properly.
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.
That looks good now and I see the new job passing now 🙌 Thanks!
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.
Oh nice! Me too TIL about Metalava! I think this looks good, other than Jay's comments. Thanks for doing this!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2370 +/- ##
=======================================
Coverage 79.92% 79.92%
=======================================
Files 284 284
Lines 10151 10151
Branches 1433 1433
=======================================
Hits 8113 8113
Misses 1422 1422
Partials 616 616 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 believe the APIs re getting mixed for both flavors. But this is close!
purchases/api-entitlement.txt
Outdated
method @kotlin.jvm.JvmSynthetic @kotlin.jvm.Throws(exceptionClasses=PurchasesException::class) public static suspend Object? awaitCustomerInfo(com.revenuecat.purchases.Purchases, optional com.revenuecat.purchases.CacheFetchPolicy fetchPolicy, kotlin.coroutines.Continuation<? super com.revenuecat.purchases.CustomerInfo>); | ||
method @kotlin.jvm.JvmSynthetic @kotlin.jvm.Throws(exceptionClasses=PurchasesTransactionException::class) public static suspend Object? awaitLogIn(com.revenuecat.purchases.Purchases, String appUserID, kotlin.coroutines.Continuation<? super com.revenuecat.purchases.data.LogInResult>); | ||
method @kotlin.jvm.JvmSynthetic @kotlin.jvm.Throws(exceptionClasses=PurchasesTransactionException::class) public static suspend Object? awaitLogOut(com.revenuecat.purchases.Purchases, kotlin.coroutines.Continuation<? super com.revenuecat.purchases.CustomerInfo>); | ||
method public static suspend Object? awaitStorefrontCountryCode(com.revenuecat.purchases.Purchases, kotlin.coroutines.Continuation<? super java.lang.String>); |
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 seems this is also including API elements from the defaults
flavor? For example, this method is only available there
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! I just added a filter for the product flavors and now it seems to work well: 925f0c1
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 🫶 this! I think only thing left is to include it to our CircleCI config
## Description Super quick follow-up to #2370. Otherwise this typo will live forever 😅
Motivation
As Toni mentioned earlier, the binary-compatibility-validator still has limitations on Android projects, particularly with support for multiple build flavors. As an alternative, we could consider using Metalava, a tool developed and maintained by Google. Fortunately, there's also a community-supported Gradle plugin available metalava-gradle which is widely adopted in popular libraries like Accompanist.
To generate Metalava signature files, you can use the base command:
However, since our project includes multiple flavors, we need to target a specific one. For example:
metalavaGenerateSignatureDefaultsRelease
metalavaGenerateSignatureCustomEntitlementComputationRelease
For compatibility checks in CI, we can use:
metalavaCheckCompatibilityDefaultsRelease
metalavaCheckCompatibilityCustomEntitlementComputationRelease
In this PR, I ran
metalavaGenerateSignatureDefaultsRelease
to generate the default signature files for both thepurchases
andpurchases-ui
modules.