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: clarify child_process.execFile{,Sync} file argument #5310

Closed

Conversation

kevinoid
Copy link
Contributor

This may be a bit of a nit, but I thought it was worth fixing for clarity.

The changes to the file argument of execFile in #4504 make it appear that execFile requires an absolute or relative path to the executable file, when in fact it also supports a filename which will be resolved using $PATH. Although the example clarifies this somewhat, assuming there isn't a node binary in $CWD, it's easy to overlook. This commit clarifies that point.

It also updates the argument description for execFileSync to match, since it was overlooked in #4504 and behaves identically.

Thanks for considering,
Kevin

The changes to the file argument of execFile in nodejs#4504 make it appear
that execFile requires an absolute or relative path to the executable
file, when it also supports a filename which will be resolved using
$PATH.  Although the example makes this clear, assuming there isn't a
node binary in $CWD, it's easy to overlook.  This commit clarifies that
point.

It also updates the argument description for execFileSync to match,
since it was overlooked in nodejs#4504 and behaves identically.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@silverwind silverwind added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Feb 18, 2016
@jasnell
Copy link
Member

jasnell commented Feb 19, 2016

LGTM

1 similar comment
@eljefedelrodeodeljefe
Copy link
Contributor

LGTM

jasnell pushed a commit that referenced this pull request Feb 20, 2016
The changes to the file argument of execFile in #4504 make it appear
that execFile requires an absolute or relative path to the executable
file, when it also supports a filename which will be resolved using
$PATH.  Although the example makes this clear, assuming there isn't a
node binary in $CWD, it's easy to overlook.  This commit clarifies that
point.

It also updates the argument description for execFileSync to match,
since it was overlooked in #4504 and behaves identically.

PR-URL: #5310
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Feb 20, 2016

Landed in 91f6bc0

@kevinoid
Copy link
Contributor Author

Great! Thanks @jasnell and @eljefedelrodeodeljefe!

@eljefedelrodeodeljefe
Copy link
Contributor

Thank you!

rvagg pushed a commit that referenced this pull request Feb 21, 2016
The changes to the file argument of execFile in #4504 make it appear
that execFile requires an absolute or relative path to the executable
file, when it also supports a filename which will be resolved using
$PATH.  Although the example makes this clear, assuming there isn't a
node binary in $CWD, it's easy to overlook.  This commit clarifies that
point.

It also updates the argument description for execFileSync to match,
since it was overlooked in #4504 and behaves identically.

PR-URL: #5310
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
The changes to the file argument of execFile in #4504 make it appear
that execFile requires an absolute or relative path to the executable
file, when it also supports a filename which will be resolved using
$PATH.  Although the example makes this clear, assuming there isn't a
node binary in $CWD, it's easy to overlook.  This commit clarifies that
point.

It also updates the argument description for execFileSync to match,
since it was overlooked in #4504 and behaves identically.

PR-URL: #5310
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 22, 2016
The changes to the file argument of execFile in #4504 make it appear
that execFile requires an absolute or relative path to the executable
file, when it also supports a filename which will be resolved using
$PATH.  Although the example makes this clear, assuming there isn't a
node binary in $CWD, it's easy to overlook.  This commit clarifies that
point.

It also updates the argument description for execFileSync to match,
since it was overlooked in #4504 and behaves identically.

PR-URL: #5310
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
The changes to the file argument of execFile in #4504 make it appear
that execFile requires an absolute or relative path to the executable
file, when it also supports a filename which will be resolved using
$PATH.  Although the example makes this clear, assuming there isn't a
node binary in $CWD, it's easy to overlook.  This commit clarifies that
point.

It also updates the argument description for execFileSync to match,
since it was overlooked in #4504 and behaves identically.

PR-URL: #5310
Reviewed-By: James M Snell <jasnell@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

Successfully merging this pull request may close these issues.

5 participants