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.

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: rephrase documentation for child_process.execSync() #14944

Closed
aashil opened this issue Aug 19, 2017 · 5 comments
Closed

doc: rephrase documentation for child_process.execSync() #14944

aashil opened this issue Aug 19, 2017 · 5 comments
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.

Comments

@aashil
Copy link
Contributor

aashil commented Aug 19, 2017

The line at the end describing about the error thrown can be worded better.

If the process times out, or has a non-zero exit code, this method will throw. The Error object will contain the entire result from child_process.spawnSync()

Should be something like:

If the process times out, or has a non-zero exit code, this method will throw an Error object which will contain the entire result from child_process.spawnSync()

https://nodejs.org/api/child_process.html#child_process_child_process_execsync_command_options

[refack adding]

If the process times out, or has a non-zero exit code, this method ***will***
throw. The [`Error`][] object will contain the entire result from
[`child_process.spawnSync()`][]

@mscdex mscdex added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Aug 19, 2017
@refack
Copy link
Contributor

refack commented Aug 19, 2017

I'd tweak the last part as well:

...
throw an [`Error`][] that will include the full result of the underlying
[`spawnSync()`][].
...
<!-- link definition in the footer -->
[`spawnSync()`]: #child_process_child_process_execsync_command_options

@refack refack added the good first issue Issues that are suitable for first-time contributors. label Aug 19, 2017
@refack
Copy link
Contributor

refack commented Aug 19, 2017

/cc @nodejs/documentation

@gibfahn
Copy link
Member

gibfahn commented Aug 19, 2017

Both suggested changes sound reasonable.

@ayazhafiz
Copy link
Contributor

I'd like to pick this up!

@refack
Copy link
Contributor

refack commented Aug 20, 2017

@ayazhafiz yours. (marked issue as in progress)
If you need any help feel free to contact me (Github @ mention, email, IM or IRC).
If you haven't already it is highly recommended to take a look at the CONTRIBUTING guide.

@refack refack added wip Issues and PRs that are still a work in progress. and removed good first issue Issues that are suitable for first-time contributors. labels Aug 20, 2017
ayazhafiz added a commit to ayazhafiz/node that referenced this issue Aug 20, 2017
Rephrases the error thrown by child_process.execSync().

Fixes: nodejs#14944
ayazhafiz added a commit to ayazhafiz/node that referenced this issue Aug 20, 2017
@refack refack removed the wip Issues and PRs that are still a work in progress. label Aug 23, 2017
addaleax pushed a commit to addaleax/ayo that referenced this issue Aug 25, 2017
Rephrases the error thrown by child_process.execSync().

PR-URL: nodejs/node#14953
Fixes: nodejs/node#14944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this issue Aug 28, 2017
Rephrases the error thrown by child_process.execSync().

PR-URL: nodejs/node#14953
Fixes: nodejs/node#14944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
Rephrases the error thrown by child_process.execSync().

PR-URL: #14953
Fixes: #14944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
Rephrases the error thrown by child_process.execSync().

PR-URL: #14953
Fixes: #14944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Sep 20, 2017
Rephrases the error thrown by child_process.execSync().

PR-URL: #14953
Fixes: #14944
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

5 participants