-
-
Notifications
You must be signed in to change notification settings - Fork 39
Adopt Swift 6 language mode and "NonisolatedNonsendingByDefault" #195
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
base: v3
Are you sure you want to change the base?
Adopt Swift 6 language mode and "NonisolatedNonsendingByDefault" #195
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #195 +/- ##
==========================================
+ Coverage 83.80% 84.74% +0.94%
==========================================
Files 28 28
Lines 3093 3061 -32
==========================================
+ Hits 2592 2594 +2
+ Misses 501 467 -34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // now we have received the PUBREL we can process the published message. PUBCOMP is sent by `respondToPubrel` | ||
| return true | ||
| } | ||
| .map { _ in publish } |
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.
This line throws the warning Reference to captured var 'publish' in concurrently-executing code.
How can we get the publishMessage out of the sendMessage's checkInbound closure?
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.
Urghh that's a horrid bit of code. That's not an easy one to resolve
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.
Not totally sure what I'm doing there. It looks like I'm trying to deal with a PUBLISH being sent twice and resetting the PUBREC/PUBREL handshake. I don't think it even works. Perhaps comment out the mutable publish and the setting of publish if we receive a publish packet add a big comment to investigate and also add an issue here to check it out.
adam-fowler
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.
There are a number of places where assumeIsolated() can be used instead of NIOLoopBound. It's easier to use and looks cleaner.
Also there are still two warnings about mutable types in MQTTConnection.connect, but I'd prefer you leave them as my NIOAsyncTestingChannel PR touches that function and I can fix in that PR to make the merge easier
Yeah, they are due to the fact that we mutate the I'm studying clean sessions from the MQTT specification because I think we're doing something wrong at the moment (we could also move The mutation of |
swift-tools-versionto 6.2There is a warning left in MQTTChannelHandler that right now I don't know how to solve and a few left in MQTTConnection that require a bit of restructuring (will do in a separate PR)
Resolves #188