- Notifications
You must be signed in to change notification settings - Fork425
Expose OS-chosen address and port; force IPv4#707
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Can you explain why this is necessary, and whether it's possible for you to make these options instead of forcing the defaults?
option( CPP-NETLIB_BUILD_SHARED_LIBS "Build cpp-netlib as shared libraries."ON ) | ||
option( CPP-NETLIB_BUILD_TESTS "Build the cpp-netlib project tests."OFF) | ||
option( CPP-NETLIB_BUILD_EXPERIMENTS "Build the cpp-netlib project experiments."OFF) | ||
option( CPP-NETLIB_BUILD_EXAMPLES "Build the cpp-netlib project examples."OFF) |
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.
I'm not comfortable with changing these defaults. Is there a reason why you don't set these from your project?
@@ -86,7 +89,7 @@ struct sync_server_base : server_storage_base, socket_options_base { | |||
using boost::asio::ip::tcp; | |||
system::error_code error; | |||
tcp::resolver resolver(service_); | |||
tcp::resolver::query query(address_, port_); | |||
tcp::resolver::query query(tcp::v4(),address_, port_); |
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.
There seems to be no reason for this -- can you not make this explicit in your code instead? As part of the options?
Hi@eile -- I sent some comments a while back and I'm unclear as to why this is necessary and why this isn't an optional thing (as opposed to something that forces IPv4). Do you mind making it so that we can configure what kind of IP addresses we'd like to get from the DNS would be? Then I can go ahead and merge the updated version. |
Hi@deanberris:@tribal-tec and me where a bit swamped lately, we'll get back to you on this. |
All good, thanks@eile -- no rush. :) |
Also by the way, I just noticed that you're attempting to make a change to the 0.11-devel branch, which we aren't actively maintaining anymore. We're trying to get everyone to use the 0.13-release branch instead. I'm happy to still merge an updated version of this in 0.11-devel, but I'm not really up for having another release on the 0.11.x line. |
tribal-tec commentedFeb 1, 2017
Please close this in favor of#729 |
No description provided.