-
Notifications
You must be signed in to change notification settings - Fork 640
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
Allow using SocketAddress
proxies instead of just InetSocketAddress
#3243
Conversation
This commit adds new socketAddress methods and tries to keep backwards compat for users using the old address methods, which are deprecated. I'd recommend removing socketAddress and renaming it back to address on the next major release. Fixes reactor#3242
I guess the error lies in that new methods to the interface were added? |
Maybe this can be fixed by keeping address unimplemented and the socketAddress method defaulting to delegating to address. And then the implementing builder class overrides socketAddress and implements address as the one delegating to socketAddress. Seems a bit unnecessary though since this class is only supposed to be implemented by the builder class. |
what do you think about this?
|
Sure, lets do it |
Should pass checks now and is ready for review. |
It seems like the build failed because of unrelated reasons with ssl stuff on windows. I've merged now upstream into my branch and I hope the build passes now. |
Failed tests on Mac OS CI are not related |
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.
@AlexProgrammerDE Thanks for the PR!
@reactor/netty-team PTAL
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.
SocketAddress
proxies instead of just InetSocketAddress
This commit adds new socketAddress methods and tries to keep backwards compat for users using the old address methods, which are deprecated. I'd recommend removing socketAddress and renaming it back to address on the next major release.
Fixes #3242