Clean up: moved obsoleted methods to a separate file#950
Conversation
81f7705 to
bd169eb
Compare
|
Thanks for the review @aboedo. I addressed all your comments and now we only have |
aee9dbd to
436c0e4
Compare
aboedo
left a comment
There was a problem hiding this comment.
looks great! Thanks for taking care of this!
There was a problem hiding this comment.
it looks like these comment blocks are redundant now, could you remove them?
There was a problem hiding this comment.
They're public methods. We could disable the lint rule for this file I guess though. What do you think?
There was a problem hiding this comment.
Ohhh I see, that makes sense.
Perhaps we could keep the old docs with a note explaining how they've been renamed?
I'm thinking about the case where someone updates to RCv4, then Xcode tells them to update the method names and they might not be sure what the methods did in the first place. If they have the old docs in place with a note, it'll be clear, but otherwise they'll just run into a deprecated method that doesn't provide much context.
What do you think?
If you did decide to add the old docs, it can be done in a separate PR so as not to hold this one.
So feel free to either keep this or disable the rule for this file for now, merge when ready, and we can continue the discussion about the docstrings in a follow-up PR. Here are the old docs for reference.
There was a problem hiding this comment.
Thanks yeah that's definitely the right thing to do! I just added those.
9af98af to
7b3fd63
Compare
7b3fd63 to
9d7c362
Compare
…yManager implementation Fixes RevenueCat#732. Instead of worrying about whether `IdentityManager.configure` was idempotent or if the implementation was correct, I realized that `Purchases` never called that method after its initialization Therefore, to simplify the whole implementation, the `configure` part is now in the constructor. Subsequent `logIn`/`logOut` calls are the only way to change the identity. Some of this code was tangled with some of the deprecations in `IdentityManager`, so to aid in this refactor, I `obsoleted` [3 methods that were already `deprecated` in version 3.13.0](https://github.com/RevenueCat/purchases-ios/blob/3.13.0/Purchases/Public/RCPurchases.h#L205-L224). This allowed me to remove the associated tests instead of changing them to use the new API. Following RevenueCat#950, these methods are now separate in `Obsoletions.swift`, and they point to the new methods. The remaining tests in `PurchasesTests` all pass, and the ones in `IdentityManagerTests` have been converted to initializing new `IdentityManager` instances. Those pass as well, so I feel confident in this refactor 💪
… `IdentityManager` implementation (#974) * Obsoleted Purchases.createAlias/identify/reset and simplified IdentityManager implementation Fixes #732. Instead of worrying about whether `IdentityManager.configure` was idempotent or if the implementation was correct, I realized that `Purchases` never called that method after its initialization Therefore, to simplify the whole implementation, the `configure` part is now in the constructor. Subsequent `logIn`/`logOut` calls are the only way to change the identity. Some of this code was tangled with some of the deprecations in `IdentityManager`, so to aid in this refactor, I `obsoleted` [3 methods that were already `deprecated` in version 3.13.0](https://github.com/RevenueCat/purchases-ios/blob/3.13.0/Purchases/Public/RCPurchases.h#L205-L224). This allowed me to remove the associated tests instead of changing them to use the new API. Following #950, these methods are now separate in `Obsoletions.swift`, and they point to the new methods. The remaining tests in `PurchasesTests` all pass, and the ones in `IdentityManagerTests` have been converted to initializing new `IdentityManager` instances. Those pass as well, so I feel confident in this refactor 💪 * SwiftAPITester: removed calls to deprecated methods * ObjCAPITester: removed calls to deprecated methods
No description provided.