Skip to content
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.

Sign up for GitHub

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

Jump to bottom

doc: simplify recommendations in process.md #42556

Merged
merged 1 commit into from
Apr 3, 2022
Merged

doc: simplify recommendations in process.md #42556

merged 1 commit into from
Apr 3, 2022

Conversation

Trott
Copy link
Member

@Trott Trott commented Apr 1, 2022

Remove recommendation that has no explanation. Make the other
recommendation less wordy.

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Apr 1, 2022
Trott
Trott commented Apr 1, 2022
View reviewed changes
doc/api/process.md
Node.js process. While it is available as a global, it is recommended to
explicitly access it via require or import:
Node.js process.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference here would be to add text explaining why the global should be avoided, but I don't know what that reason is. Also, with this change, there is now no mention of process being available as a global and that is something that should be mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mentioned here:

node/doc/api/globals.md

Lines 467 to 477 in 175638b

## `process`
<!-- YAML
added: v0.1.7
-->
<!-- type=global -->
* {Object}
The process object. See the [`process` object][] section.

I don't think we need more, especially if the intent is to promote importing/requiring the core module rather than rely on the global property.

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 3, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 3, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/42556
FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close
    at consumeBody (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Response.text (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:158:18)
    at async Request.json (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/lib/request.js:49:18)
    at async Request.query (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/lib/request.js:107:20)
    at async Request.queryAll (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/lib/request.js:134:20)
    at async Request.gql (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/lib/request.js:64:22)
    at async PRData.getComments (file:///opt/hostedtoolcache/node/16.14.0/x64/lib/node_modules/node-core-utils/lib/pr_data.js:97:21)
    at async Promise.all (index 2)
    at async Promise.all (index 1) {
  type: 'system',
  errno: 'ERR_STREAM_PREMATURE_CLOSE',
  code: 'ERR_STREAM_PREMATURE_CLOSE',
  erroredSysCall: undefined
}
https://github.com/nodejs/node/actions/runs/2084373902

@Trott
doc: simplify recommendations in process.md
b07dc4d 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: nodejs#42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
@Trott
Copy link
Member Author

Trott commented Apr 3, 2022

Landed in b07dc4d

@Trott Trott merged commit b07dc4d into nodejs:master Apr 3, 2022
@Trott Trott deleted the process2 branch April 3, 2022 06:14
juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
@Trott @juanarbol
doc: simplify recommendations in process.md
d2f17ac 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: nodejs#42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
This was referenced Apr 5, 2022
Closed
Merged
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
@Trott @juanarbol
doc: simplify recommendations in process.md
1c61418 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: #42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
@Trott @xtx1130
doc: simplify recommendations in process.md
ad21c85 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: nodejs#42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
juanarbol pushed a commit that referenced this pull request May 31, 2022
@Trott @juanarbol
doc: simplify recommendations in process.md
6dd0772 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: #42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
@juanarbol juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
@Trott @danielleadams
doc: simplify recommendations in process.md
8d4d71a 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: #42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
targos pushed a commit that referenced this pull request Jul 11, 2022
@Trott @targos
doc: simplify recommendations in process.md
ea4e127 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: #42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
@Trott @targos
doc: simplify recommendations in process.md
8850cf9 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: #42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@Trott @guangwong
doc: simplify recommendations in process.md
9cfbe1b 
Remove recommendation that has no explanation. Make the other
recommendation less wordy.

PR-URL: nodejs/node#42556
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@Mesteery Mesteery Mesteery approved these changes

@marsonya marsonya marsonya approved these changes

@benjamingr benjamingr benjamingr approved these changes

Assignees
No one assigned
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Trott @nodejs-github-bot @benjamingr @aduh95 @marsonya @Mesteery