-
Notifications
You must be signed in to change notification settings - Fork 45
Xcode26 / Swift 6 - Minimal changes for successful builds #1222
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
nolanw
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.
sorry for the delay in review, thank you very much for breaking that big pr up. let me know if you need a hand with the context.perform thing! everything else looks ok to me
| func updateLastUsedDate(for smilie: Smilie) { | ||
| guard let context = dataStore.managedObjectContext else { return } | ||
| guard let context = dataStore.managedObjectContext, | ||
| let smilieText = smilie.text else { return } |
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.
see it's funny because this is exactly wrong: we should not be pawing at managed properties outside a context.perform[AndWait]. is there any way to avoid the compiler error while only accessing smilie.text within the context.perform block?
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.
maybe the compiler can be placated by awaiting context.perform? which I guess means either introducing a Task here or marking this method async, kinda annoying.
| private var jumpToPostIDAfterLoading: String? | ||
| private var messageViewController: MessageComposeViewController? | ||
| private var networkOperation: Task<(posts: [Post], firstUnreadPost: Int?, advertisementHTML: String), Error>? | ||
| private var networkOperation: Any? |
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.
yeesh
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.
aha!
|
|
||
| /// A slightly more convenient NSManagedObject for entities with a custom class. | ||
| public class AwfulManagedObject: NSManagedObject { | ||
| public class AwfulManagedObject: NSManagedObject, @unchecked Sendable { |
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.
should probably ditch this subclass tbh. but ok for now
…objects across concurrency boundaries in updateLastUsedDate(). Access managed object properties only within context.perform block. - PostsPageViewController: Add comment explaining why networkOperation uses Any? type (Task returns non-Sendable Core Data objects; Swift 6 requires Sendable Success types).
Let's start with this. If this gets accepted then I can build on top of it with the subsequent branches
I added a line to Common.xcconfig because without it my TestFlight builds were being pushed with errors I wasn't replicating in debug builds:
SWIFT_STRICT_CONCURRENCY = minimal