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

Update deprecations #1544

Closed
wants to merge 7 commits into from
Closed

Update deprecations #1544

wants to merge 7 commits into from

Conversation

beylmk
Copy link
Contributor

@beylmk beylmk commented Apr 27, 2022

As per discussion here, we should have a single place for deprecations.

I'm not sure if it's worth making purchasesOrchestrator and that one post method internal, though. If it is, perhaps we want to add a note to make them private again once the functions are obsoleted. Opening a quick PR for discussion.

Now this is just adding back @objc to functions and adding some docs

@NachoSoto
Copy link
Contributor

I'm not sure if it's worth making purchasesOrchestrator and that one post method internal, though.

Oh yeah definitely I don't think it's worth it. I'd keep the methods that require those for sure. Sorry I didn't realize that! I don't think the trade off is worth it.

* Enable debug logging. Useful for debugging issues with the lovely team @RevenueCat.
*/
@available(*, deprecated, message: "use Purchases.logLevel instead")
@objc static var debugLogsEnabled: Bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[not for this PR] I'm surprised the others in this file don't have @objc, did we miss that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made it for this PR 🤷 since the original work got undone anyway

}

/**
* Deprecated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooff I guess we missed adding docs for this one (before this PR)

Could we add a link to https://docs.revenuecat.com/docs/user-ids and maybe a comment like

Suggested change
* Deprecated
* Deprecated: Configure behavior through the RevenueCat dashboard instead.
* Determines whether or not users may reuse a subscription from an App or Play Store account that already has that subscription active.
* ### Related articles:
* - https://docs.revenuecat.com/docs/user-ids

@@ -226,7 +226,7 @@ public typealias StartPurchaseBlock = (@escaping PurchaseCompletedBlock) -> Void
private let productsManager: ProductsManager
private let customerInfoManager: CustomerInfoManager
private let trialOrIntroPriceEligibilityChecker: TrialOrIntroPriceEligibilityChecker
private let purchasesOrchestrator: PurchasesOrchestrator
internal let purchasesOrchestrator: PurchasesOrchestrator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, I don't think this is worth the tradeoff of moving these out 🙏🏻

@beylmk beylmk changed the title Move deprecations Update deprecations Apr 27, 2022
@@ -2182,42 +2182,6 @@ class PurchasesTests: XCTestCase {
expect(invokedMethodParams.appUserID) == identityManager.currentAppUserID
}

func testAttributionDataDontSendNetworkAppUserIdIfNotProvided() throws {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these need to be removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought i was on the adservices branch 🤦‍♀️ reverted

This reverts commit 68a680d.
@@ -33,7 +33,7 @@ public extension Purchases {
@available(watchOS, introduced: 6.2, deprecated: 1, renamed: "checkTrialOrIntroDiscountEligibility(productIdentifiers:)")
@available(macOS, introduced: 10.15, deprecated: 1, renamed: "checkTrialOrIntroDiscountEligibility(productIdentifiers:)")
@available(macCatalyst, introduced: 13.0, deprecated: 1, renamed: "checkTrialOrIntroDiscountEligibility(productIdentifiers:)")
func checkTrialOrIntroDiscountEligibility(_ productIdentifiers: [String]) async -> [String: IntroEligibility] {
@objc func checkTrialOrIntroDiscountEligibility(_ productIdentifiers: [String]) async -> [String: IntroEligibility] {
Copy link
Contributor

@NachoSoto NachoSoto Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, why do these need @objc? it's an async version (so Swift only), the non-async is above, and that one doesn't have @objc?

@NachoSoto
Copy link
Contributor

I think this can be closed now?

@beylmk beylmk closed this Jun 22, 2022
@NachoSoto NachoSoto deleted the move-deprecations branch May 26, 2023 17:17
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.

3 participants