Skip to content

doc: add subprocess.ref() and subprocess.unref() methods#22220

Closed
tlhunter wants to merge 1 commit intonodejs:masterfrom
tlhunter:child-process-doc
Closed

doc: add subprocess.ref() and subprocess.unref() methods#22220
tlhunter wants to merge 1 commit intonodejs:masterfrom
tlhunter:child-process-doc

Conversation

@tlhunter
Copy link
Contributor

@tlhunter tlhunter commented Aug 9, 2018

This PR documents the subprocess.ref() and subprocess.unref() methods. The latter method does appear in the documentation as a side node in the options.detached section while the prior does not appear anywhere.

Neither takes an argument nor has a return. I copied the documentation for unref from the existing detached docs. This is now redundant, perhaps the content should be removed from detached and a link be added to ref.

The introduction version number should be correct, see git log v0.7.10 to confirm.

Checklist

@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Aug 9, 2018
bengl
bengl reviewed Aug 9, 2018
View reviewed changes
doc/api/child_process.md Outdated
Copy link
Member

@bengl bengl Aug 9, 2018

Choose a reason for hiding this comment

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

Shouldn't this be 0.7.10? And same for unref.

Copy link
Contributor Author

@tlhunter tlhunter Aug 9, 2018

Choose a reason for hiding this comment

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

Whoops! Will fix.

lundibundi
lundibundi reviewed Aug 9, 2018
View reviewed changes
doc/api/child_process.md Outdated
Copy link
Member

@lundibundi lundibundi Aug 9, 2018

Choose a reason for hiding this comment

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

Maybe `for a given `subprocess` to exit`?

Copy link
Contributor Author

@tlhunter tlhunter Aug 9, 2018

Choose a reason for hiding this comment

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

Change made

doc/api/child_process.md Outdated
Copy link
Member

@lundibundi lundibundi Aug 9, 2018

Choose a reason for hiding this comment

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

between the child and the parent?

Copy link
Contributor Author

@tlhunter tlhunter Aug 9, 2018

Choose a reason for hiding this comment

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

Change made

@tlhunter tlhunter force-pushed the child-process-doc branch from cb9e2d8 to ebbf8e6 Compare August 9, 2018 22:13
sagirk
sagirk approved these changes Aug 10, 2018
View reviewed changes
Copy link
Contributor

@sagirk sagirk left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

A few minor nits below.

doc/api/child_process.md Outdated
Copy link
Contributor

@sagirk sagirk Aug 10, 2018

Choose a reason for hiding this comment

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

nit: ... before exiting itself. reads better, IMHO.

Copy link
Contributor Author

@tlhunter tlhunter Aug 10, 2018

Choose a reason for hiding this comment

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

OK

doc/api/child_process.md Outdated
Copy link
Contributor

@sagirk sagirk Aug 10, 2018

Choose a reason for hiding this comment

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

nit: forcing instead of allowing seems to be the right word here.

Copy link
Contributor Author

@tlhunter tlhunter Aug 10, 2018

Choose a reason for hiding this comment

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

OK

doc/api/child_process.md Outdated
Copy link
Contributor

@sagirk sagirk Aug 10, 2018

Choose a reason for hiding this comment

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

nit: given the context, usage of the word process to describe parent and child can be safely omitted.

Copy link
Contributor Author

@tlhunter tlhunter Aug 10, 2018

Choose a reason for hiding this comment

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

OK

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 10, 2018

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/498/

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 10, 2018

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

@vsemozhetbyt vsemozhetbyt added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Aug 10, 2018
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 10, 2018

Landed in 7b1f3a4
Thank you!

vsemozhetbyt pushed a commit that referenced this pull request Aug 10, 2018
@tlhunter @vsemozhetbyt
doc: add subprocess.ref() and subprocess.unref()
7b1f3a4 
PR-URL: #22220
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
targos pushed a commit that referenced this pull request Aug 11, 2018
@tlhunter @targos
doc: add subprocess.ref() and subprocess.unref()
e313961 
PR-URL: #22220
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
@rvagg rvagg mentioned this pull request Aug 13, 2018
Merged
@tlhunter tlhunter deleted the child-process-doc branch August 13, 2018 17:11
This was referenced Aug 16, 2018
Closed
Closed
Closed
This was referenced Aug 16, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
firass111 pushed a commit to firass111/Project_node1 that referenced this pull request Apr 16, 2025
@tlhunter @rvagg
doc: add subprocess.ref() and subprocess.unref()
fafdae4 
PR-URL: nodejs/node#22220
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bengl bengl bengl approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

+4 more reviewers

@lundibundi lundibundi lundibundi left review comments

@TimothyGu TimothyGu TimothyGu approved these changes

@sagirk sagirk sagirk approved these changes

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.