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

Pass Channels down pipeline not NWConnection #45

Merged
merged 1 commit into from
May 22, 2019

Conversation

Lukasa
Copy link
Collaborator

@Lukasa Lukasa commented May 22, 2019

Motivation:

The expectation is that server channels use Channel as their data type,
but the initial data type in NIOTSListenerChannel was actually
NWConnection. This is unnecessary and it makes it hard to interop
between NIOTS and NIO.

Modifications:

  • Initialize the NIOTSConnectionChannel in the NIOTSListenerChannel
    instead of in AcceptHandler.
  • Added some missing @available annotations in the tests.

Result:

More consistency
Resolves #44.

@Lukasa Lukasa added the semver/patch No public API change. label May 22, 2019
@Lukasa Lukasa added this to the 1.0.3 milestone May 22, 2019
@Lukasa Lukasa requested a review from weissi May 22, 2019 15:19
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Looks great! but maybe add a test?

@Lukasa
Copy link
Collaborator Author

Lukasa commented May 22, 2019

Pfft, fine. 😆

Motivation:

The expectation is that server channels use Channel as their data type,
but the initial data type in NIOTSListenerChannel was actually
NWConnection. This is unnecessary and it makes it hard to interop
between NIOTS and NIO.

Modifications:

- Initialize the NIOTSConnectionChannel in the NIOTSListenerChannel
instead of in AcceptHandler.
- Added some missing @available annotations in the tests.

Result:

More consistency
@Lukasa
Copy link
Collaborator Author

Lukasa commented May 22, 2019

Ok, test added that inserts something into the server channel initializer that expects a channel instead of an NWConnection.

@Lukasa Lukasa requested a review from weissi May 22, 2019 15:37
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

👍 thanks

@Lukasa Lukasa merged commit d59370d into apple:master May 22, 2019
@Lukasa Lukasa deleted the cb-channel-is-a-thing branch May 22, 2019 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NIOTSListenerBootstrap incompatible with NIOExtras.ServerQuiescingHelper
2 participants