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: killSignal option accepts integer values #10424

Closed

Conversation

thefourtheye
Copy link
Contributor

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc child_process

Description of change

killSignal option accepts the signal name or signal number as well.

`killSignal` option accepts the signal name or signal number as well.
@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. lts-watch-v6.x labels Dec 23, 2016
@sam-github
Copy link
Contributor

Is this behaviour covered by tests?

@thefourtheye
Copy link
Contributor Author

@sam-github There are few cases in test/parallel/test-child-process-spawnsync-validation-errors.js, but #10423 has enough cases I believe.

@sam-github
Copy link
Contributor

Are you saying #10424 is documentation for #10423? If so, merge the docs into the PR that introduces the API being documented.

@thefourtheye
Copy link
Contributor Author

@sam-github This behaviour is already there. #10423 improves the validation.

@sam-github
Copy link
Contributor

I suggest putting docs and tests in the same commit in the future, it makes them "obviously correct".

But LGTM as is.

@jasnell
Copy link
Member

jasnell commented Dec 29, 2016

This is not landing cleanly in master despite this not showing any conflicts here. Can you rebase and update the PR nevermind! appears to have been a temporary glitch on my end

jasnell pushed a commit that referenced this pull request Jan 6, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 6, 2017

Landed in fc647fd

@jasnell jasnell closed this Jan 6, 2017
@thefourtheye thefourtheye deleted the doc-killsignal-type-update branch January 7, 2017 00:51
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 19, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
thefourtheye added a commit to thefourtheye/io.js that referenced this pull request Feb 5, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
thefourtheye added a commit to thefourtheye/io.js that referenced this pull request Feb 5, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 6, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 6, 2017
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@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.

6 participants