doc: add a cli options doc page #5787
Conversation
| Specify ICU data load path. (overrides `NODE_ICU_DATA`) | ||
|
|
||
|
|
||
| ## ENVIRONMENT VARIABLES |
There was a problem hiding this comment.
Does this need to be all caps?
|
LGTM with a couple comments. |
Fishrock123
force-pushed
the
doc-cli-options
branch
2 times, most recently
from
70ac750
to
5326d92
Compare
|
LGTM |
|
|
||
| <!--type=misc--> | ||
|
|
||
| Node.js come with a wide variety of CLI options with multiple ways to execute scripts, built-in debugging, and a handful of other helpers. |
There was a problem hiding this comment.
Suggestion:
Node.js comes with a wide variety of CLI options. These options expose multiple ways to execute scripts, built-in debugging, and other helpful runtime options.
|
Line wrapping is somewhat inconsistent. Perhaps wrap it at 80? |
Fishrock123
force-pushed
the
doc-cli-options
branch
from
5326d92
to
96f6b9a
Compare
|
|
||
| <!--type=misc--> | ||
|
|
||
| Node.js comes with a wide variety of CLI options. These options expose built-in debugging, multiple ways to execute scripts, and other helpful runtime options. |
There was a problem hiding this comment.
@chrisdickinson Worded it this way so that it didn't sound like "multiple" applied to everything.
|
@bengl Fixed. I forgot about that when copying from the man page. |
|
@Fishrock123 lines 5 and 154? |
Fishrock123
force-pushed
the
doc-cli-options
branch
from
96f6b9a
to
accc590
Compare
|
Oops, didn't notice my editor was wrapping there, thought I had inserted spaces. Fixed. |
|
Cool, LGTM! |
|
Yep, nice. LGTM. |
|
Awesome work on all the recent docs PRs lately @Fishrock123 ! |
|
|
||
| <!--type=misc--> | ||
|
|
||
| Node.js comes with a wide variety of CLI options. These options expose built-in |
There was a problem hiding this comment.
CLI options -> command line options perhaps? Are we sure all the users are familiar with the term? Windows users in particular...
There was a problem hiding this comment.
I was hoping that the title would clarify it, but I suppose I can can do this.
|
LGTM with nits. To clarify none of the nits prevent the merge IMO and they can be addressed at a later point or now depending on @Fishrock123's opinion. |
|
I would have liked if some of this feedback was done when I wrote some of these in the man doc refactor. D: |
|
I'd be against the nits, since it will become inconsistent to |
|
To be clear - I'm in the same opinion of @eljefedelrodeodeljefe of land now merge later. |
|
ok landing this as-is and making a new updating PR I'm afraid for this there isn't a better way to keep it in sync, formatting differences and other nits vary quite a bit from the |
|
Actually, I'm going to add to the comment in https://github.com/nodejs/node/blob/master/src/node.cc#L3296 in this PR also, just to try to help keep them in sync. |
Fishrock123
force-pushed
the
doc-cli-options
branch
from
accc590
to
f10894b
Compare
|
Ci, since this now modifies node.cc with a comment: https://ci.nodejs.org/job/node-test-pull-request/1992/ |
|
(Also I added a comment about |
This page is mostly a mirror of the updated manual page. PR-URL: nodejs#5787 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell jasnell@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>
Fishrock123
force-pushed
the
doc-cli-options
branch
from
f10894b
to
d0edabe
Compare
|
Oops. Landed in 91cf55b, will make a new PR to fix some of the docs in multiple spots like tomorrow. |
This page is mostly a mirror of the updated manual page. PR-URL: #5787 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell jasnell@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>
This page is mostly a mirror of the updated manual page. PR-URL: #5787 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell jasnell@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>
This page is mostly a mirror of the updated manual page. PR-URL: #5787 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell jasnell@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>
Pull Request check-list
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
existing APIs, or introduces new ones)?
Affected core subsystem(s)
doc
Description of change
This page is mostly a mirror of the updated manual page. This came about as a result of this twitter conversation, and I realized there wasn't an easy way to get the options documentation from the website.
cc @nodejs/documentation
Edit: Rendered (but our css makes it look slightly different)