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

Rename possibly offensive terminology in child_process #14444

Closed
1 of 3 tasks
benjamingr opened this issue Jul 24, 2017 · 13 comments
Closed
1 of 3 tasks

Rename possibly offensive terminology in child_process #14444

benjamingr opened this issue Jul 24, 2017 · 13 comments
Labels
child_process Issues and PRs related to the child_process subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core.

Comments

@benjamingr
Copy link
Member

benjamingr commented Jul 24, 2017

Currently, we use the terminology "Child died" when child processed get terminated.

We discussed the terminology in #14293 (comment) , copying my comment from there:

I'm usually the last one to worry about that sort of stuff - but is it possible this terminology is offensive? If we swapped out other trigger references I think it would be nice to have a code base without dead children.

@jasnell suggested we change the terminology to "Child exited". I agree it's better terminology, in general I think we should avoid irrelevant stuff that might cause people to feel uneasy (like death related terminology in code) where we can and I think it could help keep the project friendly and inclusive.


  • Estimate the impact of changing the error message. (Semver Major?)
  • Check for any objections from collaborators to this change.
  • Establish consensus that these changes are something we want to do (vs, collaborators feel they are churn).
@benjamingr benjamingr added child_process Issues and PRs related to the child_process subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Jul 24, 2017
@benjamingr
Copy link
Member Author

Impact

Since it is internal code, I think we can just remove it safely from here (and other places). To be on the safe side I did a GitHub search. Looking at https://github.com/search?l=JavaScript&q=%22Child+died%22&type=Code&utf8=%E2%9C%93 - there certainly are usages of the terminology (or have a fork of node in their repo), but nothing relying on anything internal (I went through the 'child dead' search too). I don't see anyone relying on it or using it any way. (Warning, the search results contain some jokes one might consider very offensive).

I'm confident we can do it as semver-patch in all cases that are test related. Any objections?

@TimothyGu
Copy link
Member

+1 for semver-patch.

@sam-github
Copy link
Contributor

Sounds good, just make sure not to use "exited" unless it specifically refers to a process exiting. Processes (on unix) can terminate via exit, or terminate via signal. Terminate is the generic technical term used in the man pages to cover both cases, see discussion of WIFEXITED vs WIFSIGNALED in http://man7.org/linux/man-pages/man2/waitpid.2.html.

@fastman
Copy link

fastman commented Jul 26, 2017

Hi,
When we are on this, what do you think about replacing
child.kill([signal]) [1]
with
child.sendSignal([signal]) ?

Killing the child just does not sound right :/

[1] https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_child_kill_signal

@bnoordhuis
Copy link
Member

@fastman It maps 1-to-1 to the system call of the same name.

@nodejs nodejs deleted a comment from xenon2 Jul 26, 2017
@benjamingr
Copy link
Member Author

@fastman

When we are on this, what do you think about replacing

I'm with @bnoordhuis here, while I'm not a fan of the name unix chose for signals - kill is the operating system commend and I don't think we can replace it. I'd rather avoid offensive terminology as much as possible - but not at the expense of changing operating system terms.

@fastman
Copy link

fastman commented Jul 27, 2017

@benjamingr
I understand, but should't we strive to be a better community?
In my opinion, saying that something was done wrong in the past, sticking to it and repeating mistakes should not be a way to go.

Maybe at least we could change the wording in the documentation? [1]

@bnoordhuis
I'm not sure that it maps 1-1 to the system call - I think that kill function in Node.JS does a little bit more.

[1] https://linux.die.net/man/2/kill

@TimothyGu
Copy link
Member

sticking to it and repeating mistakes should not be a way to go

Nor should creating a set of "Node.js-specific terminology" against established tradition be our modus operandi, IMHO. There are multiple duties (in a deontological sense) at play here:

  1. The wording should not be disturbing.
  2. Maintaining compatibility with older Node.js versions.
  3. Maintaining consistent terminology to help developers familiar with other environments transition to Node.js.

Where we draw the line between 1 and 2/3 is of course the good ol' problem of virtue ethics.

The OP does not have concern 2), which is a big reason why I am in support of it. It also does not correspond to a syscall or UNIX command, rather derived artificially.

The "suicide" flag did have concern 2) in play, but since again it does not correspond to a syscall, and the word is somewhat more disturbing than kill (IMO) it passed the threshold.

Of course, this is all subjective, but as multiple members of the community have shown the proposal does have objections against it.

I would be okay with adding a sendSignal alias as that is the more precise name, but am -1 against deprecating or removing kill. Whether Node.js's kill function is a vanilla wrapper of the syscall, or does it do a bit more, is too semantical to be relevant IMO.

@fastman
Copy link

fastman commented Jul 27, 2017

@TimothyGu What do you think about changing the wording in the documentation? child variable can be replaced with any name.
kill(2) manual is more precise in that matter and avoids confusing terminology. Apart from the name itself it is about sending signal to a process

+1 for the alias of course!

@benjamingr
Copy link
Member Author

I'd like to see the discussion about the possibility of renaming .kill or adding an alias moved to another (new) thread if you're fine with it @fastman .

@refack
Copy link
Contributor

refack commented Aug 20, 2017

@benjamingr has this progressed? If not might I suggest a solution similar to #14578 — replace the term child with subprocess then the verb becomes way less significant.

@benjamingr
Copy link
Member Author

@refack that sounds like a good idea. To be honest I completely forgot about this issue 0_0

If you want to (and have time) to PR this be my guest - otherwise probably next weekend.

@benjamingr
Copy link
Member Author

This was resolved a while ago.

@refack refack removed their assignment Oct 12, 2018
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. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

No branches or pull requests

6 participants