-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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: fix added:
info for some methods
#42661
Conversation
Review requested:
|
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.
LGTM
doc/api/http.md
Outdated
added: | ||
- v8.0.0 | ||
- v7.7.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.
So this feature landed on v7.7.0? Why do we want to add v8.0.0 on this list? I don't think that's an interesting addition, all features of v7.7.0 are on v8.0.0 (or we should list all releases that happen since v7.7.0).
added: | |
- v8.0.0 | |
- v7.7.0 | |
added: v7.7.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.
No, the methods were added to v8.0.0 and backported to v7.7.0. See PR description and commit message.
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.
We do the same for other backports.
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.
Do you mean that since there are no "holes" it does not make sense to list v8.0.0? In that case I see your point but the info is a bit misleading.
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.
There are other occurrences like this that we might want to fix if you have a strong opinion about this:
Personally, I think this is more correct.
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.
Node.js 8.0.0 was released on 2017-05-30, Node.js 7.7.0 was released 3 months before that on 2017-02-28. I don't see how it's relevant to our user if the feature landed on the Current release line because of a backport or as a regular cherry-picking, the backport doesn't make it special IMO.
That's different, Node.js v6.13.0 was a LTS release, so it makes sense to list it there as it first landed on v7.0.0.
That's an interesting one, v4.2.0 is listed as an LTS release (with the --check
change)
node/doc/changelogs/CHANGELOG_V4.md
Lines 3069 to 3077 in 86099a3
## 2015-10-07, Version 4.2.0 'Argon' (LTS), @jasnell | |
### Notable changes | |
The first Node.js LTS release! See <https://github.com/nodejs/LTS/> for details of the LTS process. | |
* **icu**: Updated to version 56 with significant performance improvements (Steven R. Loomis) [#3281](https://github.com/nodejs/node/pull/3281) | |
* **node**: | |
* Added new `-c` (or `--check`) command line argument for checking script syntax without executing the code (Dave Eddy) [#2411](https://github.com/nodejs/node/pull/2411) |
So one could think that v5.0.0 was the Current release that landed first, but it looks like it was released after v4.2.0, and --check
change is not listed on its changelog:
node/doc/changelogs/CHANGELOG_V5.md
Line 1591 in 86099a3
## 2015-10-29, Version 5.0.0 (Stable), @rvagg |
So yes, I think we should fix 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.
lgtm
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d.
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.
lgtm
Landed in 4508c8c |
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d. PR-URL: nodejs#42661 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Qingyu Deng <i@ayase-lab.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d. PR-URL: #42661 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Qingyu Deng <i@ayase-lab.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d. PR-URL: #42661 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Qingyu Deng <i@ayase-lab.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d. PR-URL: #42661 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Qingyu Deng <i@ayase-lab.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d. PR-URL: #42661 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Qingyu Deng <i@ayase-lab.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
`outgoingMessage.getHeader()`, `outgoingMessage.getHeaderNames()`, and `outgoingMessage.getHeaders()` were added to Node.js v7.7.0 via 3e8d43d165ef. PR-URL: nodejs/node#42661 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mestery <mestery@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Qingyu Deng <i@ayase-lab.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
outgoingMessage.getHeader()
,outgoingMessage.getHeaderNames()
, andoutgoingMessage.getHeaders()
were added to Node.js v7.7.0 via3e8d43d.