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: make execFileSync in line with execFile #2940

Closed
wants to merge 2 commits into from

Conversation

lfortin
Copy link
Contributor

@lfortin lfortin commented Sep 18, 2015

Make execFileSync API docs in line with execFile API docs.

make execFileSync API docs in line with execFile API docs
@evanlucas
Copy link
Contributor

LGTM

@brendanashworth brendanashworth added the doc Issues and PRs related to the documentations. label Sep 18, 2015
@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Sep 18, 2015
@silverwind
Copy link
Contributor

@lfortin while you're at it, could you move the whole execFileSync section below exexSync so the ordering is the same as for the async methods?

@lfortin
Copy link
Contributor Author

lfortin commented Sep 22, 2015

@silverwind done.

@silverwind
Copy link
Contributor

@lfortin The diff looks a bit unwieldy, this is just a cut-paste job, right?

@lfortin
Copy link
Contributor Author

lfortin commented Sep 22, 2015

@silverwind yes it is. I confirm that the text is well formatted.

@silverwind
Copy link
Contributor

Alright, LGTM.

@@ -735,10 +735,15 @@ 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`](#child_process_child_process_spawnsync_command_args_options)

[EventEmitter]: events.html#events_class_events_eventemitter
[net.Server]: net.html#net_class_net_server
[net.Socket]: net.html#net_class_net_socket
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure these should have moved. Isn't there a convention to put the link definitions at the end of the file ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, good catch. These should indeed remain an the end of the file, @lfortin.

@lfortin
Copy link
Contributor Author

lfortin commented Sep 22, 2015

@targos I see, my bad. I will re-issue a new commit with the link definitions at the end of the file.

/cc @silverwind

@lfortin
Copy link
Contributor Author

lfortin commented Sep 22, 2015

@targos @silverwind done.

@@ -766,6 +767,7 @@ 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`](#child_process_child_process_spawnsync_command_args_options)


Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry but this looks like an unnecessary newline.

rearrange execSync, execFileSync API docs to be in line with exec, execFile API docs
@lfortin
Copy link
Contributor Author

lfortin commented Sep 22, 2015

@silverwind unnecessary new line removed.

@silverwind
Copy link
Contributor

LGTM

1 similar comment
@targos
Copy link
Member

targos commented Sep 22, 2015

LGTM

silverwind pushed a commit that referenced this pull request Sep 23, 2015
PR-URL: #2940
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
silverwind pushed a commit that referenced this pull request Sep 23, 2015
Changed the ordering so it is in line with the async methods.

PR-URL: #2940
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@silverwind
Copy link
Contributor

Thanks, landed in 28da400 and 2d21092.

@silverwind silverwind closed this Sep 23, 2015
Fishrock123 pushed a commit that referenced this pull request Sep 25, 2015
PR-URL: #2940
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Sep 25, 2015
Changed the ordering so it is in line with the async methods.

PR-URL: #2940
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
This was referenced Sep 30, 2015
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.

6 participants