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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: child_process.kill() on windows needs clarification #34858

Closed
1 task
s4m0r4m4 opened this issue Aug 20, 2020 · 4 comments 路 Fixed by #34867
Closed
1 task

doc: child_process.kill() on windows needs clarification #34858

s4m0r4m4 opened this issue Aug 20, 2020 · 4 comments 路 Fixed by #34867
Labels
doc Issues and PRs related to the documentations.

Comments

@s4m0r4m4
Copy link

馃摋 API Reference Docs Problem

  • Version: v14 and previous ones as well, like v12 LTS
  • Platform: Windows
  • Subsystem: child_process signals

Location

Section of the site where the content exists

Affected URL(s):

Description

Concise explanation of the problem

The NodeJS signals documentation (https://nodejs.org/api/process.html#process_signal_events) is pretty clear that on windows child_process.kill() does NOT actually send the signal, but rather just kills the child process abruptly.

However, the child_process.kill() documentation does not make this clarification, and it's opening sentence ("The subprocess.kill() method sends a signal to the child process. ") leads one to believe that the signal will in fact be sent to the child process on any platform, windows included.

I'd love to just see a quick blurb/sentence that clarifies the behavior on windows vs other platforms (or perhaps just a link pointing to the signals documentation - https://nodejs.org/api/process.html#process_signal_events).


  • I would like to work on this issue and
    submit a pull request.
@s4m0r4m4 s4m0r4m4 added the doc Issues and PRs related to the documentations. label Aug 20, 2020
@s4m0r4m4 s4m0r4m4 changed the title doc: child_process.kill() on windows misleading doc: child_process.kill() on windows needs clarification Aug 20, 2020
@joaolucasl
Copy link
Contributor

I can work on this if you're not willing to submit the PR yourself @s4m0r4m4, would that be okay?

@s4m0r4m4
Copy link
Author

Totally! I'm haven't contributed to the NodeJS docs before so I'm fine with you doing it!

@juanarbol
Copy link
Member

Thank you @s4m0r4m4 for your report, amazing, and thank you @joaolucasl for offering to help. Leave a comment if you need help.

@joaolucasl
Copy link
Contributor

@juanarbol @s4m0r4m4

Just opened a PR with an initial paragraph to make this clearer. Let me know with a comment there if you have suggestions :)

Trott pushed a commit to joaolucasl/node that referenced this issue Jan 8, 2021
Clarify the inner workings of .kill on Windows,
since termination signals are not available there.

Fixes: nodejs#34858
joaolucasl added a commit to joaolucasl/node that referenced this issue Mar 26, 2021
Clarify the inner workings of .kill on Windows,
since termination signals are not available there.

Fixes: nodejs#34858
@Trott Trott closed this as completed in 504ed7c Mar 30, 2021
MylesBorins pushed a commit that referenced this issue Apr 4, 2021
Clarify the inner workings of .kill on Windows,
since termination signals are not available there.

Fixes: #34858

PR-URL: #34867
Reviewed-By: Juan Jos茅 Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
Clarify the inner workings of .kill on Windows,
since termination signals are not available there.

Fixes: #34858

PR-URL: #34867
Reviewed-By: Juan Jos茅 Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants