Uh oh!
There was an error while loading. Please reload this page.
Add documentation line for host:port invocation - #28405
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Trott
left a comment
There was a problem hiding this comment.
Welcome @sleekweasel and thanks for the pull request. This looks good to me once the hard-coded IP address is removed per one of @richardlau's suggestions.
Co-Authored-By: Richard Lau <riclau@uk.ibm.com>
sleekweasel
commented
Jun 25, 2019
Argh, I can't believe I conflated localhost with the 192 network! It was a stressful, frustrating day. |
| stream.write( | ||
| 'Usage: node [options] [ -e script | script.js | - ] [arguments]\n' + | ||
| ' node inspect script.js [arguments]\n\n' + | ||
| ' node inspect [script.js | -e "script" | <host>:<port>] …\n\n' + |
There was a problem hiding this comment.
This uses an ellipsis character (…) where everywhere else, we use three dots (...). On the one hand, the ellipsis is more correct. On the other hand, is there a situation where it will be an unprintable character because the terminal only accepts ASCII? Regardless, the code should probably pick one or the other and stick with it.
There was a problem hiding this comment.
Linter requires non-ASCII characters (to keep binary size down, believe it or not) so ... it must be.
Also, this should probably align with line 166 in that no " around script and -e comes before the script.js option.
Assuming you left the default checked state for the checkbox about allowing collaborators in this repo to push to your branch, I'll make these updates right now....
Trott
left a comment
There was a problem hiding this comment.
LGTM with or without the ellipsis comment being addressed. (It can be addressed in a later pull request if it doesn't get addressed in this one. Might even be a good first contribution for someone.)
BridgeAR
commented
Jul 4, 2019
BridgeAR
commented
Jul 4, 2019
The commit message should be updated either before landing or while landing and it should include the subsystem (Seems like |
Trott
commented
Jul 6, 2019
I got rid of |
Trott
commented
Jul 6, 2019
Trott
commented
Jul 12, 2019
Landed in 7008d78 |
PR-URL: nodejs#28405 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: #28405 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
I just wasted a load of time not knowing how to invoke 'node inspect' with host:port to debug a process started with --inspect-port=0 because it didn't show up from a lot of Googling. Having it in the help text would have saved that.