-
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: add added:
data for cli.md
#6960
Conversation
@@ -154,53 +208,80 @@ Note: v8 options allow words to be separated by both dashes (`-`) or underscores | |||
For example, `--stack-trace-limit` is equivalent to `--stack_trace_limit`. | |||
|
|||
### `--tls-cipher-list=list` | |||
<!-- YAML | |||
added: v4.0.0 |
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.
Didn't this get backported or something?
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.
It looks like that may have been in 0.12 or whatever, but it wasn't in any of the io.js releases, so... I don't know, I think it reduces confusion if we say 4.0.0. Otherwise people need the historic knowledge that 1.0.0-3.x.x are special. I'd prefer to assume people don't know that.
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.
Yes, this originally landed in v0.12 and did not come into any of the io.js streams. It landed in v4 as part of the convergence.
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 think that means that 4.0.0 is the right version to go with in the YAML comment. Does anyone disagree? /cc @addaleax @bengl @sam-github
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.
@Trott +1 for keeping it as 4.0.0… the doctool would support adding multiple versions here, but I implemented that more with LTS backports in mind, and I think adding some 0.12.x here would be more confusing than helping.
lgtm minus the nit |
Generally LGTM |
LGTM |
PR-URL: nodejs#6960 Refs: nodejs#6578 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Landed in 9067581 |
PR-URL: nodejs#6960 Refs: nodejs#6578 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: nodejs#6960 Refs: nodejs#6578 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Checklist
Affected core subsystem(s)
doc
Description of change
Added YAML info to
cli.md
. (Also removed one unnecessary word from the text because it was bugging me.)Refs: #6578