Fix child process docs#8639
Fix child process docs#8639sam-github wants to merge 7 commits intonodejs:v0.10from sam-github:fix-child-process-docs
Conversation
|
ping |
There was a problem hiding this comment.
Should probably split the two clauses -- "cwd specifies the working directory of the process to be spawned. If not given, the default is the working directory of the parent process".
|
Great work! I feel that some parts may need a little clarification / modification; but I'm generally +1 on this change. |
|
Thanks for the comments, @chrisdickinson, I'll fix up and ping you. |
|
@chrisdickinson ping |
There was a problem hiding this comment.
You don't like the "delete the entire line" option?
|
Thanks for bearing with me on this :) |
The example is not correct, and hasn't been since at least v0.10. Exec errors are emitted as an error event, and documented as such in the 'error' event section.
env was documented as not being process.env under some condtions, which is no longer correct.
Default was perhaps obvious, but not documented.
The lack of hyperlinks requires scrolling past pages of examples to find the options documentation, the default value of the 'stdio' option was not documented, and the documentation was not ordered readably.
Someone must have been confused by how pipes work, but this explanatory text is worse than nothing, it implies that ending stdin will cause child processes to terminate, which isn't true for the vast majority of them.
|
OK, I rebased, and removed the text describing some of the mechanisms child node processes can use to interact with their parent. Its not specific to child_process, its not clear to me if pipe even works on Windows for non-stdio, so I'm not the one to describe that feature, at least ATM. The rest of the PR doesn't depend on this paragraph. |
|
This looks great to me! Thanks very much. Going to leave this open till EOD (5PM PST) tomorrow for other folks to look at, and if there are no objections, I'll merge then. |
|
I looked at the changes a while ago. No comments. |
- Add hyperlinks from spawn options to subsections detailing what
those options do.
- Clarify some verbiage around ChildProcess.prototype.std{in,out,err}.
- Remove second-person pronoun.
PR-URL: #8639
Reviewed-by: Chris Dickinson <[email protected]>
|
Merged in 3a08b7c. Thanks! |
|
@chrisdickinson thanks for your careful proof-reading. FWIW, |
|
@chrisdickinson Just to confirm, this will hit v0.12 eventually, right? My assumption is that PRs should be targetted at the oldest maintained branch they apply to, but I don't want this lost. |
- Add hyperlinks from spawn options to subsections detailing what
those options do.
- Clarify some verbiage around ChildProcess.prototype.std{in,out,err}.
- Remove second-person pronoun.
PR-URL: nodejs#8639
Reviewed-by: Chris Dickinson <[email protected]>
- Add hyperlinks from spawn options to subsections detailing what
those options do.
- Clarify some verbiage around ChildProcess.prototype.std{in,out,err}.
- Remove second-person pronoun.
PR-URL: nodejs/node-v0.x-archive#8639
Reviewed-by: Chris Dickinson <[email protected]>
Cherry-picked-from: nodejs/node-v0.x-archive@3a08b7c3e0bc6757e70889196
4 participants
Fix a number of outright errors in child process documentation, as well as improve the general readability of it by using hyperlinks, documenting defaults, ordering the docs consistently, etc.