-
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: simplify text in pull-requests.md #30458
Conversation
PR-URL: nodejs#30346 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Add documentation for the `lookup` option. PR-URL: nodejs#30353 Fixes: nodejs#30171 Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes: nodejs#30211 PR-URL: nodejs#30356 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
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. I would like to point out though that the former description of step 4. is easier for me to follow as a non-native speaker.
PR-URL: nodejs#29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#29053 Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#30249 Fixes: nodejs#30237 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
Let's fix this to maintain that ease-of-understanding. Is this better?:
|
The "maintenance is supported by" stuff in BUILDING.md is unclear. It seems unnecessary so I propose removing it. I don't understand what it means to, in this context, support maintenance. Does it mean that you simply do the maintenance? Does that mean it really just means "maintain"? Do we really mean that we mantain support, rather than support maintenance? That information does not seem necessary to include. I'm not sure it's meaningful. With (for example) Windows, is it accurate to say that the Node.js core team maintains support for it? Or is it accurate to say that support is maintaned by smaller groups or individuals within the Node.js core team? Both could be considered accurate. So is the difference meaningful? I think the more important elements of determinig tier support are the other listed elements. PR-URL: nodejs#30365 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The statement that tests for tier 1 platforms are run on multiple variants is not true. We usually only run on one variant of macOS. Remove "multiple", which simplifies and clarifies the statements anyway. PR-URL: nodejs#30366 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Mostly, this replaces "It is recommended to do X" with "Do X." PR-URL: nodejs#30458 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Landed in 794d8be |
Mostly, this replaces "It is recommended to do X" with "Do X." PR-URL: #30458 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Mostly, this replaces "It is recommended to do X" with "Do X." PR-URL: #30458 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Mostly, this replaces "It is recommended to do X" with "Do X." PR-URL: #30458 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Mostly, this replaces "It is recommended to do X" with "Do X."
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes