-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Translate a few simple Core Data model types into Swift #24983
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
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 29771 | |
| Version | PR #24983 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 1f275c2 | |
| Installation URL | 0eg2pc1pqe4e0 |
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 29771 | |
| Version | PR #24983 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 1f275c2 | |
| Installation URL | 545khcjihurho |
14b925c to
2f647ff
Compare
jkmassel
left a comment
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 looks good to me – thanks for the clear commits – that make it a lot easier to review.
| return "Category" | ||
| } | ||
|
|
||
| @objc public static var uncategorizedId: NSNumber { |
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'm not too sure about uncategorizedId but I understand why it works this way – doing PostCategory.uncategorized would imply that it's a PostCategory object not the default ID.
Probably not worth addressing?
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 have change the name in 1f275c2 to make it consistent with the original ObjC code: PostCategoryUncategorized(objc) -> PostCategory.uncategorized(swift)
| // MARK: - Constants | ||
|
|
||
| public let PostCategoryEntityName = "Category" | ||
| public let PostCategoryNameKey = "categoryName" |
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.
| // MARK: - Constants | |
| public let PostCategoryEntityName = "Category" | |
| public let PostCategoryNameKey = "categoryName" |
I don't see these referenced anywhere so I think they can be removed? None of the other classes have them.
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.
Good catch. Deleted in dd8b359.
|
|
||
| @interface PostCategory : NSManagedObject | ||
|
|
||
| @property (nonatomic, strong) NSNumber *categoryID; |
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 changed from optional to non-optional in this PR, but I validated that it's not optional in the Core Data model so the change is valid.
|





Description
Translate a few simple Core Data model types into Swift. I hope to eliminate Objective-C code in the WordPressData module, so that we have the option to move it to the SPM package.
It'd be easier to review this PR commit by commit.