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

Fix concurrency warnings #424

Merged
merged 8 commits into from
Jul 16, 2024
Merged

Conversation

GeorgeElsham
Copy link
Contributor

An alternative to #422.

Fixes #418.

@GeorgeElsham
Copy link
Contributor Author

One thing I notice that's a bit different to #422's attempt is the Package.swift. Is there any information on what this is (being having a 6.0 version) about and is it needed / should I also include it here?

@Alex293
Copy link
Contributor

Alex293 commented Jun 24, 2024

@GeorgeElsham I think you used the Swift6 compiler in Swift 5 mode, this definitively not build on my side with Swift 6

@Alex293
Copy link
Contributor

Alex293 commented Jun 24, 2024

What I did with the Package.swift file allows working with previous compiler versions and in both languages versions. Note that there is Package@swift-5.7.0 too.

@GeorgeElsham
Copy link
Contributor Author

@Alex293 You're right, I see. I'll try to fix it, thank you.

@Alex293
Copy link
Contributor

Alex293 commented Jun 24, 2024

I think we should gather the opinion of the author and the community if there is interest in using some of the constructs outside of SwiftUI. If not, I don’t think there is much sense in spending effort going further than what I did in the other branch. Did you identify any use cases for non main actor but sendable apis in this repo ?

@Alex293
Copy link
Contributor

Alex293 commented Jun 24, 2024

@davdroman can I ask your opinion ? Do you know about any potential use of this lib APIs outside of of SwiftUI/MainActor ?

@davdroman
Copy link
Collaborator

@Alex293 not off the top of my head, I'd find it strange that anyone would want to employ introspection in a background thread.

@GeorgeElsham
Copy link
Contributor Author

@Alex293 Unfortunately I've had pretty limited time, but I've fixed the issues. It still uses @MainActor only when appropriate. I've tested using Xcode 15.4 Beta and Xcode 16.0 Beta. Let me know if there are still any problems.

@c0dedbear
Copy link

Hello guys, when are you planning to merge this into the main? From Xcode 16 Beta 3 and Swift 6 mode those warnings are errors.

@Alex293
Copy link
Contributor

Alex293 commented Jul 9, 2024

I can't speak for the maintainers so no merge ETA from me but you can use my fork in the mean time (I'd be glad if you could at least try it to see if there are any thing I missed).

Last time I checked, this branch still missed some annotations for things being spi or similar, I can't remember. TBH I'm slightly annoyed to review this branch as I already did the work on another branch and asked which direction to pursue before continuing investing time on any of the branches. I'll wait for the maintainers to chime in and try to contribute any whatever direction they prefer.

@c0dedbear
Copy link

I can speak for the maintainers so no merge ETA from me but you can use my fork in the mean time (I'd be glad if you could at least try it to see if there are any thing I missed).

Last time I checked, this branch still missed some annotations for things being spi or similar, I can't remember. TBH I'm slightly annoyed to review this branch as I already did the work on another branch and asked which direction to pursue before continuing investing time on any of the branches. I'll wait for the maintainers to chime in and try to contribute any whatever direction they prefer.

It's seems like your fixes have eliminated errors and warnings. btw stay tuned for maintainer(s)...

@GeorgeElsham GeorgeElsham marked this pull request as ready for review July 15, 2024 17:54
@davdroman davdroman merged commit 9068014 into siteline:main Jul 16, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swift 6 complete concurrency warnings
4 participants