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

Update CI to use release 5.10 image #47

Merged
merged 2 commits into from
Mar 11, 2024
Merged

Conversation

glbrntt
Copy link
Contributor

@glbrntt glbrntt commented Mar 8, 2024

No description provided.

@glbrntt glbrntt added the semver/none No version bump required. label Mar 8, 2024
@simonjbeaumont
Copy link
Collaborator

@swift-server-bot add to allowlist

@glbrntt
Copy link
Contributor Author

glbrntt commented Mar 8, 2024

5.10 is failing because strict concurrency checking and warnings as errors are enabled.

The concurrency checking emits a warning because of the generated test file which contains a static array of test classes, and XCTestCase isn't Sendable.

Do you have a preference for which option to disable?

@czechboy0
Copy link
Collaborator

czechboy0 commented Mar 11, 2024

contains a static array of test classes

Maybe we can just move it to be a fileprivate global let in the file, to avoid coupling it to the (non-Sendable) XCTestCase subclass?

Sorry, misread your message. Yeah, not sure. @briancroom what's the recommendation here?

@simonjbeaumont
Copy link
Collaborator

Do you have a preference for which option to disable?

I thought I'd check the runtime and generator repos for precedence here and found that we have disabled strict-concurrency in one and warnings-as-errors in the other 🤦.

Ideally we'd disable neither but don't have a strong opinion about the stopgap solution. Let's make sure we file an issue to cover getting back to belt-and-braces. The longer it's disabled, the harder it's gonna be to enable again.

//cc @czechboy0

@glbrntt
Copy link
Contributor Author

glbrntt commented Mar 11, 2024

I disabled strict concurrency checking for now and filed #49 to re-enable it.

I also filed a radar against SwiftPM.

Copy link
Collaborator

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Thanks @glbrntt!

@czechboy0 czechboy0 merged commit f81270e into apple:main Mar 11, 2024
5 checks passed
@glbrntt glbrntt deleted the update-ci branch March 11, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants