-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
doc: improve server.listen() random port #7976
Conversation
port value of `0` to have the operating system assign an available port. | ||
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise. | ||
Omit the port argument to have the operating system assign a random port, | ||
which can later be retrieved in `server.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.
s/in/using
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.
"later" is when? :)
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.
@ronkorving hah, good catch! Went with after the 'listening' event has been emitted
instead.
A few nits but generally LGTM! Thank you! |
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise. Use a | ||
port value of `0` to have the operating system assign an available port. | ||
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise. | ||
Omit the port argument to have the operating system assign a random 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.
I don't think we should completely remove the fact that 0
can be used. Could we combine the existing content and your changes?
LGTM with a comment. |
591a94e
to
862a84f
Compare
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise. Use a | ||
port value of `0` to have the operating system assign an available port. | ||
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise. | ||
Omit the port argument or use a port value of `0` to have the operating system |
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.
Small nit: Omit the port argument, or use a port value of
0, to have ...
(here and below)
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.
Fixed, thanks!
One additional nit... otherwise LGTM! |
Minor rewording related to making a server listen to a random port, and added how to retrieve which port was randomly chosen by the OS. Also changed documented `server.listen()` signature as it does in fact not require `port` to be provided. PR-URL: nodejs#7976 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
862a84f
to
57e95d9
Compare
CI resulted in some broken node-test-binary-arm tests which I can't imagine is related to a simple doc change, so this is good to go. |
Those addon tests do extract code from docs, so they could conceivably be affected by a doc change, but I don't think the docs edited here are the ones involved. Uh, but re-running CI again anyway: https://ci.nodejs.org/job/node-test-commit/4421/ |
@Trott huh, got any pointers to what that is exactly so I can read up on that for later? |
|
Thank you very much @phillipj. CI is green! |
Minor rewording related to making a server listen to a random port, and added how to retrieve which port was randomly chosen by the OS. Also changed documented `server.listen()` signature as it does in fact not require `port` to be provided. PR-URL: #7976 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Landed in 66af6a9 |
Minor rewording related to making a server listen to a random port, and added how to retrieve which port was randomly chosen by the OS. Also changed documented `server.listen()` signature as it does in fact not require `port` to be provided. PR-URL: #7976 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Minor rewording related to making a server listen to a random port, and added how to retrieve which port was randomly chosen by the OS. Also changed documented `server.listen()` signature as it does in fact not require `port` to be provided. PR-URL: nodejs#7976 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Affected core subsystem(s)
doc
Description of change
Minor rewording related to making a server listen to a random port, and added how to retrieve which port was randomly chosen by the OS. Although the previous text ".. to have the operating system assign an available port" is correct, I think using the term "random port" is easier to spot and usually what one might be searching for when looking for this type of functionality.
Also changed documented
server.listen()
signature as it does in fact not requireport
to be provided (lib/net.js#L1342)./cc @nodejs/documentation