Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adopt Sendable in NIOExtras #174

Merged
merged 3 commits into from
Aug 23, 2022
Merged

Adopt Sendable in NIOExtras #174

merged 3 commits into from
Aug 23, 2022

Conversation

dnadoba
Copy link
Member

@dnadoba dnadoba commented Aug 23, 2022

Incremental Sendable adoption.

Incremental `Sendable` adoption.
@dnadoba dnadoba added the 🆕 semver/minor Adds new public API. label Aug 23, 2022
public var cumulationBuffer: ByteBuffer? {
get { nil }
set { /* discard newValue */ }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we can safely make this behavioural change. Given that there's no particular reason to require this type to be Sendable, I propose we mark it "explicitly not Sendable" and tolerate that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, that's the better solution. Shortly thought about making FixedLengthFrameDecoder a struct instead of a class but that would be a source breaking change too.

Changed in daae52c

@Lukasa Lukasa enabled auto-merge (squash) August 23, 2022 12:45
@Lukasa Lukasa merged commit 5334d94 into apple:main Aug 23, 2022
@dnadoba dnadoba deleted the dn-sendable-extras branch August 23, 2022 14:59
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants