Uh oh!
There was an error while loading. Please reload this page.
doc: improve server.listen() random port - #7976
Conversation
There was a problem hiding this comment.
@ronkorving hah, good catch! Went with after the 'listening' event has been emitted instead.
jasnell
commented
Aug 4, 2016
A few nits but generally LGTM! Thank you! |
There was a problem hiding this comment.
I don't think we should completely remove the fact that 0 can be used. Could we combine the existing content and your changes?
cjihrig
commented
Aug 4, 2016
LGTM with a comment. |
There was a problem hiding this comment.
Small nit: Omit the port argument, or use a port value of0, to have ...
(here and below)
jasnell
commented
Aug 5, 2016
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>
phillipj
commented
Aug 5, 2016
phillipj
commented
Aug 5, 2016
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. |
Trott
commented
Aug 5, 2016
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/ |
phillipj
commented
Aug 5, 2016
@Trott huh, got any pointers to what that is exactly so I can read up on that for later? |
Trott
commented
Aug 5, 2016
|
jasnell
commented
Aug 5, 2016
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>
phillipj
commented
Aug 5, 2016
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 requireportto be provided (lib/net.js#L1342)./cc @nodejs/documentation