feat: upgrade to swift 6.0 #30
Conversation
williazz
requested review from
ArielDemarco,
bryce-b,
nachoBonafonte and
vvydier
as code owners
| XCTAssert(ActivityContextManager.instance.getCurrentContextValue(forKey: .span) === span1) | ||
| let expec = expectation(description: "testStartAndEndSpanInAsyncQueue") | ||
| DispatchQueue.global().async { | ||
| let span2 = self.createSpan(parentSpan: span1, name: "testStartAndEndSpanInAsyncQueue2") |
There was a problem hiding this comment.
create span and endSpanAndValidateContext not only start and end the span but also validate the active span before and after running, and that is not being done now. We should keep calling them as that validation is very key for the tests, probably the methods need updating to something like:
static nonisolated func createSpan( tracer: DefaultTracer, parentSpan: Span, name: String) static nonisolated func endSpanAndValidateContext(span: Span, parentSpan: Span?)
And be called properly, even wrapping the passing span in a Mutex or using the Locked class from the SDK that allows similar functionality to Mutex
|
Except for that detail with the tests all the rest looks really good, you have done a great work. |
Revision 2Originally, I deleted SemanticAttributes file because it's deprecated. But I realized the migration to SemanticConventions file contains a lot of breaking changes, which should be reviewed separately. Also addressed the issue @nachoBonafonte raised by restoring the ActivityContextManagerTests |
|
Did some manual testing against AwsHackerNewsDemo using Simulator, iPhone 16, and iPhone 13 running Swift 5.9. My setup was
Images
|
|
please pull main to get the updated workflow |
3 participants
Revision 1
Swift 6.0 adds some concurrency checks. This was some bitter work but I completed the migration work for core.
Implementation
@unchecked Sendableto pretty much every classTesting
I removed the strict checks on test code to keep code changes minimal. I still need to do some manual end-to-end testing to get some more confidence.
Revision 2
This is standalone PR that is compatible with projects using Swift v5.9
Manually tested against real devices - See #30 (comment)