-
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
Require experimental annotation for trusted entitlements API #1066
Require experimental annotation for trusted entitlements API #1066
Conversation
@@ -54,6 +54,7 @@ data class EntitlementInfo( | |||
val billingIssueDetectedAt: Date?, | |||
val ownershipType: OwnershipType, | |||
private val jsonObject: JSONObject, | |||
@ExperimentalPreviewRevenueCatPurchasesAPI |
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.
Originally I tried to add this annotation to the VerificationResult
and EntitlementVerificationMode
enums themselves, but that made it so the whole class required the annotation. So in the end I only added to the APIs where those enums may be used.
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.
Yeah, this is due to RequiresOptIn
Contagiousness.
b9c9d6f
to
41d997c
Compare
api-tester/src/main/java/com/revenuecat/apitester/java/PurchasesAPI.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## bring-back-trusted-entitlements-api #1066 +/- ##
======================================================================
Coverage ? 85.64%
======================================================================
Files ? 176
Lines ? 6249
Branches ? 860
======================================================================
Hits ? 5352
Misses ? 563
Partials ? 334 |
* | ||
* Default mode is disabled. | ||
*/ | ||
@JvmSynthetic |
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.
This makes it so this method is only available in Kotlin. The alternative is to have it enabled in Java but then there won't be any compiling issue if users try to use this method inadvertently.
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.
Are we planning on releasing this on the hybrids before it's not experimental?
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 I don't think we will release it in the hybrids while it's experimental... IMO, we should wait until we are confident in the natives before porting to the hybrids. Lmk if you think otherwise though!
41d997c
to
0d23243
Compare
14b7431
to
f731745
Compare
@@ -106,6 +107,7 @@ data class EntitlementInfo( | |||
get() = jsonObject | |||
|
|||
/** @suppress */ | |||
@OptIn(ExperimentalPreviewRevenueCatPurchasesAPI::class) |
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.
This is not transitive right? meaning that calling toString
doesn't require to opt in as well
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's correct. If you use @OptIn
, it doesn't propagate. It would if I just annotated with the @ ExperimentalPreviewRevenueCatPurchasesAPI
again
Glad to see that we're leveraging #1065 to mark APIs as experimental 😊 |
47eef03
to
ed0380c
Compare
Description
This PR has changes from #1060 and is based on #1061
This adds a require opt in requirement to use the trusted entitlements API.