-
Notifications
You must be signed in to change notification settings - Fork 55
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 library to work with TCA 0.57 #35
Conversation
This change updates all references to `Effect` to `EffectPublisher`, and cleans up a few warnings with `LocationManager.failing` by using the more recent `XCTUnimplemented` function, though it doesn't update the signature to use a function at this time. This should let the library compile for versions of tca >= 0.43, including 0.57 which broke the old 2-generic clause `Effect` I've confirmed that tests pass on tca 0.43 and 0.57, but not on versions in between
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.
Thanks for taking this on! We should definitely have a supported version for the final 0.x series of TCA. Just one change I think we should consider and then we can look into merging soon!
stopMonitoringVisits: { .failing("LocationManager.stopMonitoringVisits") }, | ||
stopUpdatingHeading: { .failing("LocationManager.stopUpdatingHeading") }, | ||
stopUpdatingLocation: { .failing("LocationManager.stopUpdatingLocation") } | ||
accuracyAuthorization: XCTUnimplemented("LocationManager.accuracyAuthorization"), |
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 think we should revert the use of XCTUnimplemented
since it changes the behavior a bit: .failing
returns an effect that emits XCTFailures, but XCTUnimplemented
will crash with a fatal error if no default return value is specified.
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.
Sure thing, I can do that shortly!
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.
Thanks!
Is there a plan to migrate this to use the 1.0.0 version? |
From a build log seems like CI's simulator destination needs to be updated to iPhone 14 simulator instead of iPhone 11 Pro Max? Do you need help to land it? cc @stephencelis |
…ations that will fail to compile in xcode 15
Hoping things pass now; I modifies the example code+tests to also build with the newest non-1.0 tca and tweaked the makefile to use newer tv/watch simulators for safety as well. I had to remove a bunch of |
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.
Thanks for working through this!
* Update library to work with TCA 0.57 This change updates all references to `Effect` to `EffectPublisher`, and cleans up a few warnings with `LocationManager.failing` by using the more recent `XCTUnimplemented` function, though it doesn't update the signature to use a function at this time. This should let the library compile for versions of tca >= 0.43, including 0.57 which broke the old 2-generic clause `Effect` I've confirmed that tests pass on tca 0.43 and 0.57, but not on versions in between * Undo changes to failing client * Match swift tools verstion that swift-composable-architecture uses * Update ci.yml * update makefile to use iphone 14 simulator * fix build issues with example workspace, and remove unavailable annotations that will fail to compile in xcode 15 * Fix example test builds * wip --------- Co-authored-by: Stephen Celis <[email protected]> Co-authored-by: Stephen Celis <[email protected]>
This change updates all references to
Effect
toEffectPublisher
, and cleans up a few warnings withLocationManager.failing
by using the more recentXCTUnimplemented
function, though it doesn't update the signature to use a function at this time. This should let the library compile for versions of tca >= 0.43 (whenEffectPublisher
was introduced), including 0.57 which broke the old 2-generic clauseEffect
.I've confirmed that tests pass on tca 0.43 and 0.57, but not on versions in between.