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

Migrate RCPurchaseOwnershipType #594

Merged
merged 14 commits into from
Jun 25, 2021
Merged

Migrate RCPurchaseOwnershipType #594

merged 14 commits into from
Jun 25, 2021

Conversation

taquitos
Copy link
Contributor

Resolves #558
Starts work on EntitlementInfo (#553)

  • Migrates RCAppStore
  • Migrates RCPeriodType
    Temporary holding place created ReOrgLater

- Added an `APITester` target that relies on the project's public API
- Added build phase run step that builds the `APITester` after the `Purchases` framework
- If the `APITester` target fails, that means we changed the public API
Circular dependencies... let's just do a new circle CI step for the target.
Forgot to update this dependency 🤦‍♂️
Because apparently this is a thing.
- Added an `APITester` target that relies on the project's public API
- Added build phase run step that builds the `APITester` after the `Purchases` framework
- If the `APITester` target fails, that means we changed the public API
Circular dependencies... let's just do a new circle CI step for the target.
@taquitos taquitos mentioned this pull request Jun 25, 2021
@taquitos taquitos linked an issue Jun 25, 2021 that may be closed by this pull request
@taquitos taquitos requested a review from a team June 25, 2021 20:00
Base automatically changed from issue_559 to swift_migration June 25, 2021 20:44
@@ -3,6 +3,8 @@
// Copyright © 2019 RevenueCat. All rights reserved.
//

@import PurchasesCoreSwift;
Copy link
Contributor

Choose a reason for hiding this comment

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

would we eventually not need the #import "RCEntitlementInfo.h" ? if you have a favorite resource describing swift/obj c enum compatibility, let me know 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's exactly right, eventually all will be in swift, and we won't need any @import inside our SDK. This is just temporary while we move it all over.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, once we have everything in a single swift module, we don't need an import at all - everything within the same Swift module is auto-imported as part of the same scope (assuming the privacy is internal or public)

Copy link
Member

Choose a reason for hiding this comment

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

as for resources, Apple has a few pages that are pretty thorough (but sadly they don't go very deep into migrating frameworks):
https://developer.apple.com/documentation/swift/migrating_your_objective-c_code_to_swift
https://developer.apple.com/documentation/swift/imported_c_and_objective-c_apis

Copy link
Member

Choose a reason for hiding this comment

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

😂 and I just realized I was too slow for this one

Purchases/Public/RCEntitlementInfo.h Show resolved Hide resolved
@@ -746,6 +742,7 @@
2DCB85BF2406EC3F003C1260 /* Recovered References */ = {
isa = PBXGroup;
children = (
37E3564466002C9162AC7C5E /* Package.swift */,
Copy link
Member

Choose a reason for hiding this comment

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

it got moved to Recovered References, does it look fine in the project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strangely, it looks fine.

@@ -14,6 +14,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface RevenueCatAPI<RCPurchasesDelegate> : NSObject

+ (void)allTheThings;
Copy link
Member

Choose a reason for hiding this comment

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

perhaps we should rename this one since it doesn't actually cover all the things...
How about methodsAndPropertiesInPurchases?

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 have a change queued up for this too... ha

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PublicSDKAPITester/RevenueCatAPI.m Show resolved Hide resolved
Comment on lines +8 to +10
#import <Purchases/RCEntitlementInfo.h>
#import <Purchases/RCPurchaserInfo.h>
#import <Purchases/RCPurchases.h>
Copy link
Member

Choose a reason for hiding this comment

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

would doing @import Purchases; or #import <Purchases/Purchases.h> cover it?
I think this app should mimic a regular app as closely as possible

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 have this change also queued up in another branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Purchases.xcodeproj/project.pbxproj.orig Outdated Show resolved Hide resolved
path = Public;
sourceTree = "<group>";
};
B3DDB55A26854A0A008CCF23 /* ReOrgLater */ = {
Copy link
Member

Choose a reason for hiding this comment

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

Why ReOrgLater?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since they are EntitlementInfo and EntitlementInfos related, I was going to wait to address that, then organize

@taquitos taquitos merged commit a6564db into swift_migration Jun 25, 2021
@taquitos taquitos deleted the 558 branch June 25, 2021 21:57
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.

Migrate RCPurchaseOwnershipType
3 participants