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.

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

doc: add warning about Windows process groups #3681

Closed
wants to merge 1 commit into from
Closed

doc: add warning about Windows process groups #3681

wants to merge 1 commit into from

Conversation

r-52
Copy link
Contributor

@r-52 r-52 commented Nov 5, 2015

This commit adds a warning for Windows platforms. process.kill wont kill a process group on Windows and instead it throws an error.

Ref.-Issue: #3617

@mscdex mscdex added doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform. process Issues and PRs related to the process subsystem. labels Nov 5, 2015
@pmq20
Copy link
Contributor

pmq20 commented Nov 11, 2015

IMHO, this should be added to known issues rather than being a documented behaviour. We might be able to implement this in the future to make it work on Windows.

@bnoordhuis
Copy link
Member

I think people are more likely to read the documentation than the release notes when something doesn't work. This PR LGTM FWIW but "Ref.-Issue:" should be "Refs:" in the commit log.

@r-52
Copy link
Contributor Author

r-52 commented Nov 11, 2015

@pmq20 I thought that the docs are probably a more prominent place for such a warning/ disclaimer - just in case that someone wont read the full changelog.
@Fishrock123 looks like the next release is still in a proposal state, maybe you could add a reference ( #3617) to the "Know Issues"?

@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

@romankl ... can you please rebase and update?

@r-52
doc: add warning about Windows process groups
8859f54 
This commit adds a warning for Windows platforms. `process.kill` wont
kill a process group on Windows and instead it throws an error.

Refs: nodejs#3617
@r-52
Copy link
Contributor Author

r-52 commented Nov 13, 2015

@jasnell done - thanks for the reminder!

@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

LGTM!

r-52 added a commit that referenced this pull request Nov 13, 2015
@r-52 @jasnell
doc: add warning about Windows process groups
406c596 
This commit adds a warning for Windows platforms. `process.kill` wont
kill a process group on Windows and instead it throws an error.

Refs: #3617
PR-URL: #3681
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 13, 2015

Landed in 406c596

@jasnell jasnell closed this Nov 13, 2015
@r-52 r-52 deleted the windows-kill branch November 13, 2015 21:22
r-52 added a commit that referenced this pull request Nov 17, 2015
@r-52 @Fishrock123
doc: add warning about Windows process groups
d137f0f 
This commit adds a warning for Windows platforms. `process.kill` wont
kill a process group on Windows and instead it throws an error.

Refs: #3617
PR-URL: #3681
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rvagg rvagg mentioned this pull request Dec 17, 2015
Closed
r-52 added a commit that referenced this pull request Dec 29, 2015
@r-52
doc: add warning about Windows process groups
595320a 
This commit adds a warning for Windows platforms. `process.kill` wont
kill a process group on Windows and instead it throws an error.

Refs: #3617
PR-URL: #3681
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
@r-52
doc: add warning about Windows process groups
70c95ea 
This commit adds a warning for Windows platforms. `process.kill` wont
kill a process group on Windows and instead it throws an error.

Refs: #3617
PR-URL: #3681
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
Merged
This was referenced Mar 29, 2020
Open
Open
Open
Open
Open
This was referenced Mar 30, 2020
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@r-52 @pmq20 @bnoordhuis @jasnell @mscdex @MylesBorins