Skip to content

doc: change type of child_process.signalCode to string #35223

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 24, 2020
Merged

doc: change type of child_process.signalCode to string #35223

merged 1 commit into from
Sep 24, 2020

Conversation

SimplyLinn
Copy link

@SimplyLinn SimplyLinn commented Sep 16, 2020 •
edited
Loading

During testing, and interfacing with child_processes, I found that the child_proccess.signalCode property to be the string representation of the signal, not number.

Tested on node v14.6.0 (Windows)
Tested on node v14.9.0 (Linux)

Sample code:

const child_process = require('child_process');
const cp = child_process.spawn('sleep', ['10']);
cp.kill();
setTimeout(() => console.log(cp.signalCode, typeof cp.signalCode), 100);
Checklist

@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. labels Sep 16, 2020
@SimplyLinn SimplyLinn mentioned this pull request Sep 16, 2020
Merged
10 tasks
@SimplyLinn SimplyLinn changed the title doc: change type of child_proccess.signalCode to string doc: change type of child_process.signalCode to string Sep 16, 2020
@SimplyLinn
Copy link
Author

Further testing confirms the same behavior on:

node v6.17.1 (linux)
node v7.10.1 (linux)
node v8.17.0 (linux)
node v9.11.2 (linux)
node v10.22.1 (linux)
node v11.15.0 (linux)
node v12.18.4 (linux)
node v13.14.0 (linux)

addaleax
addaleax approved these changes Sep 16, 2020
View reviewed changes
@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 21, 2020
@Trott
doc: change type of child_process.signalCode to string
e36ffb7 
During testing, and interfacing with child_processes, I found that the
child_process.signalCode property to be the string representation of
the signal, not number.

PR-URL: nodejs#35223
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott
Copy link
Member

Trott commented Sep 24, 2020

Landed in e36ffb7.

Thanks for the contribution! 🎉

@Trott Trott merged commit e36ffb7 into nodejs:master Sep 24, 2020
MylesBorins pushed a commit that referenced this pull request Sep 24, 2020
@MylesBorins
doc: change type of child_process.signalCode to string
b9d767c 
During testing, and interfacing with child_processes, I found that the
child_process.signalCode property to be the string representation of
the signal, not number.

PR-URL: #35223
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 29, 2020
Merged
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@joesepi
doc: change type of child_process.signalCode to string
3fcfea7 
During testing, and interfacing with child_processes, I found that the
child_process.signalCode property to be the string representation of
the signal, not number.

PR-URL: nodejs#35223
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@SimplyLinn SimplyLinn deleted the patch-1 branch March 11, 2021 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SimplyLinn @Trott @addaleax @nodejs-github-bot