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

NetServer rework #1771

Merged
merged 3 commits into from
Jan 20, 2017
Merged

NetServer rework #1771

merged 3 commits into from
Jan 20, 2017

Conversation

vietj
Copy link
Member

@vietj vietj commented Jan 20, 2017

Provide the NetServerBase that manages the state of a TCP server that is extracted from NetServerImpl in order to reuse it in vertx-mqtt-server that will benefit of its feature. The vertx-mqtt-server is based on Netty's MQTT codec.

* @return the net server metrics SPI
*/
TCPMetrics<?> createMetrics(NetServer server, SocketAddress localAddress, NetServerOptions options);
TCPMetrics<?> createMetrics(SocketAddress localAddress, NetServerOptions options);
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you checked if this is used in Metrics impl ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if it's not, a breaking change notice is mandatory

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I'll record a line for that in change log

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it is used because NetServer only provides useless stuff : listen / close / handler, so it should be fine.

@vietj vietj merged commit 434fe92 into master Jan 20, 2017
@vietj vietj removed the to review label Jan 20, 2017
@vietj vietj deleted the net-rework branch May 14, 2018 14:23
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.

2 participants