-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Update child_process.md #50370
Update child_process.md #50370
Conversation
doc/api/child_process.md
Outdated
@@ -842,7 +842,7 @@ pipes between the parent and child. The value is one of the following: | |||
file, socket, or a pipe with the child process. The stream's underlying | |||
file descriptor is duplicated in the child process to the fd that | |||
corresponds to the index in the `stdio` array. The stream must have an | |||
underlying descriptor (file streams do not until the `'open'` event has | |||
underlying descriptor (file streams do not open until the `'open'` event has |
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'm not sure if "open" is the correct term here, maybe "start"? @nodejs/streams @nodejs/child_process wdyt?
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'm not a native speaker but "start" seems better. However, I think "open" is fine too.
doc/api/child_process.md
Outdated
@@ -842,7 +842,7 @@ pipes between the parent and child. The value is one of the following: | |||
file, socket, or a pipe with the child process. The stream's underlying | |||
file descriptor is duplicated in the child process to the fd that | |||
corresponds to the index in the `stdio` array. The stream must have an | |||
underlying descriptor (file streams do not until the `'open'` event has | |||
underlying descriptor (file streams do not open until the `'open'` event has |
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'm not a native speaker but "start" seems better. However, I think "open" is fine too.
@joseph0007 can you please fix the commit message? See https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md#commit-message-guidelines. |
Could you lint the first commit? You can use |
Ok will change the commit message in a bit. |
I won't be able to change the commit message, it will have to be done by someone with access. |
Here's how I fixed it: git fetch https://github.com/nodejs/node.git main
git reset FETCH_HEAD --hard
curl -L https://github.com/nodejs/node/pull/50370.patch | git am
# here I changed `open` to `start` as suggested in the comment above
git commit --amend -m 'doc: add missing word in `child_process.md`'
git push git@github.com:joseph0007/node.git HEAD:patch-1 -f |
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
@mcollina Please refrain from starting Jenkins CI on doc-only PR, it's not required and a waste of time (both for Jenkins hosts, and volunteers who can't use the CQ while the Jenkins result is pending). |
Landed in fa95289 |
That is the only way to land PRs using the commit queue. I’ll refrain from starting it and landing docs PRs from now on. (I do not have time to manually land them, as I mostly do these light reviews from mobile). |
Ah my mistake. At some point it wasn’t. I’ll try it next time. |
PR-URL: nodejs#50370 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
PR-URL: nodejs#50370 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
PR-URL: #50370 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
PR-URL: #50370 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
PR-URL: #50370 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
child_process: grammatical issue was fixed with the sentence.