Skip to content

Remove NIOProtocolNegotiationResult#2554

Merged
Lukasa merged 1 commit intoapple:mainfrom
FranzBusch:fb-remove-protocol-negotiation-result
Oct 16, 2023
Merged

Remove NIOProtocolNegotiationResult#2554
Lukasa merged 1 commit intoapple:mainfrom
FranzBusch:fb-remove-protocol-negotiation-result

Conversation

@FranzBusch
Copy link
Copy Markdown
Member

Motivation

After playing around more with the new async bootstrap methods, I came to the conclusion that the NIOProtocolNegotiationResult isn't carrying its weight. The NIOProtocolNegotiationResult is a glorified EventLoopFuture with an easy way to recursively resolve nested futures. Furthermore, it forces any nested protocol negotiation to use the same generic type for the end result.

Modification

This PR removes the NIOProtocolNegotiationResult and changes all the tests to be solely based on EventLoopFutures.

Result

Less code to support the async bootstrap and better composition of nested protocol negotiation handlers.

@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Oct 16, 2023
# Motivation
After playing around more with the new async bootstrap methods, I came to the conclusion that the `NIOProtocolNegotiationResult` isn't carrying its weight. The `NIOProtocolNegotiationResult` is a glorified `EventLoopFuture` with an easy way to recursively resolve nested futures. Furthermore, it forces any nested protocol negotiation to use the same generic type for the end result.

# Modification
This PR removes the `NIOProtocolNegotiationResult` and changes all the tests to be solely based on `EventLoopFuture`s.

# Result
Less code to support the async bootstrap and better composition of nested protocol negotiation handlers.
@FranzBusch FranzBusch force-pushed the fb-remove-protocol-negotiation-result branch from 9575062 to c0abb3f Compare October 16, 2023 10:56
@FranzBusch FranzBusch enabled auto-merge (squash) October 16, 2023 10:56
@FranzBusch FranzBusch disabled auto-merge October 16, 2023 10:56
@FranzBusch
Copy link
Copy Markdown
Member Author

API breakage is expected. @Lukasa do you mind merging this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants