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

replace suicide as a verb #3721

Closed
MylesBorins opened this issue Nov 9, 2015 · 86 comments
Closed

replace suicide as a verb #3721

MylesBorins opened this issue Nov 9, 2015 · 86 comments
Labels
cluster Issues and PRs related to the cluster subsystem.

Comments

@MylesBorins
Copy link
Contributor

As discussed with @mikeal and @jasnell in #3720

we could alias 'suicide' to another word in the codebase to maintain reverse compatibility while moving to the new word and updating the docs. then it's a feature rather than compat break and stays semver-minor. @mikeal

What word should we use instead?


Content warning: some discussions of suicide in the comments below. --@isaacs

@silverwind
Copy link
Contributor

Where does it say we shouldn't use it? In the same vein, one could say kill is bad, which is an universally used term in the Unix world.

@Qard
Copy link
Member

Qard commented Nov 9, 2015

I'm 👍 on replacing problematic terminology gradually, with aliases. For the kill and suicide terms, it seems to me like stop and stopped actually makes more sense, especially across language barriers. The master and worker terminology could also be parent and child, which seems a bit friendlier.

Sadly, the whole unix process model is overridden with terminology related to violence and slavery... 😞

@bnoordhuis
Copy link
Member

Don't forget the sexual overtones: mount, touch, finger... Back on topic, we can't remove the .suicide property because of backwards compatibility. I'm -1 on aliasing it, that's just more surface area for bugs and confusion.

@mikeal
Copy link
Contributor

mikeal commented Nov 9, 2015

I'm -1 on kill because it's documented that way in syscalls so this would add a lot of confusion, same for anything else that is documented as such far below the node.js layer. suicide is purely part of our own abstraction and should be changed. This goes back to the Node.js is fun principal: seeing suicide in my code bums me out and is decidedly not fun.

@mscdex
Copy link
Contributor

mscdex commented Nov 9, 2015

Yeah IMHO I'm not seeing the benefit of changing the wording here when there would be backwards incompatibility. The word is short and to the point. As @silverwind mentioned, this is a slippery slope and in general, deviating from traditional UNIX/industry word usage (e.g. renaming kill to stop) is likely to just cause more confusion.

I'm -1 as well.

@mikeal
Copy link
Contributor

mikeal commented Nov 9, 2015

Can you re-scope this to just be about suicide rather than opening up the potentially endless bikeshed about every verb in the code base?

@ChALkeR
Copy link
Member

ChALkeR commented Nov 9, 2015

-1 for aliasing unless the reason for doing that is very serious.

That term suicide there is not directed onto someone, and without that, words are just words. You can see that word in any dictionary, in Wikipedia, in some book, etc. — it doesn't make the dictionary, Wikipedia, or the book bad.

Also, It's already there, non broken, and is purposely exposed. It's also not a security nor a bugfix. That would make deprecation a semver-major change, I guess. And I doubt that it will go away in the forseeable future.

@mikeal
Copy link
Contributor

mikeal commented Nov 9, 2015

Yeah IMHO I'm not seeing the benefit of changing the wording here when there would be backwards incompatibility.

The whole point is that we can do this in a reverse compatible way without breaking any code, so this is just not true.

I think that this verb is particularly meaningful in that a member of our community killed himself not that long ago and those of us that were close to him would rather not be reminded of it every time we use this API.

@mscdex mscdex added the cluster Issues and PRs related to the cluster subsystem. label Nov 9, 2015
@jasnell
Copy link
Member

jasnell commented Nov 9, 2015

Going back and changing everything that is potentially unfortunate or problematic is likely not worth the effort. The comment I made that kicked this conversation off in the first place was due to my forgetting that suicide was already a term in use in the code. I'd rather not add to the problem by introducing new stuff. However, if we we have the opportunity to make improvements as we go, then we should likely do so, but we shouldn't bikeshed on it too much or try to solve everything all at once.

@mscdex
Copy link
Contributor

mscdex commented Nov 9, 2015

Yeah IMHO I'm not seeing the benefit of changing the wording here when there would be backwards incompatibility.

At some point it would be removed though and if other parts of our API have been any indication, not everyone updates their code for things like this and sometimes we end up being "stuck" with past decisions.

I think that this verb is particularly meaningful in that a member of our community killed himself not that long ago and those of us that were close to him would rather not be reminded of it every time we use this API.

While that is tragic, to me it's important to separate technical, personal, historical, etc. contexts here and the suicide term as we use it in node.js is used in a purely technical context. I'd be much more open to changing the wording if for example the word was not an accurate description of what it was representing.

Also, as I said, changing this one particular instance leads to a slippery slope because then we would have to evaluate the entire code base and somehow objectively determine the "offensiveness" of each word we use because of the large diversity of negative things that exist in the world. For example, what about a female coder that had an abortion and has an issue calling sub-processes, "child processes" because the world "child" reminds her of her abortion and to add insult to injury, node's built-in module is also called child_process, requiring her to see the word every time she goes to start a subprocess? What about child.kill()? What about process.kill()? The list goes on...

Deviating from existing technical terms used throughout common computing platforms and the computing industry in general because a separation of contexts cannot be made on an individual level will IMHO cause more problems that will outweigh any benefit that may come of it.

That's my 2 cents.

@mikeal
Copy link
Contributor

mikeal commented Nov 9, 2015

@mscdex There's a big gap between potentially offensive verbs we are using, and will in all likelihood continue to use, because they are adopted from the layer below us (like kill) and those that we are adopting in APIs that are entirely Node.js. In this case, we actually own this abstraction, we are not perpetuating the use of a common term, in the case of suicide we've actually chosen it for an API. This term isn't even common, I've never seen it anywhere else, a quick google for "suicide API" found no technical pages and a lot of depressing real stories, so I'm not seeing the case for persisting this verb when it's easily in our ability to move past it without negatively impacting users. You're making a good case for several other verbs, just not this one.

@mikeal
Copy link
Contributor

mikeal commented Nov 9, 2015

At some point it would be removed though

Only if nobody is using it anymore, the cost of keeping the compatibility code there is basically zero.

@bnoordhuis
Copy link
Member

the cost of keeping the compatibility code there is basically zero.

There is no such thing as zero cost code.

@mikeal
Copy link
Contributor

mikeal commented Nov 9, 2015

"basically zero" == "as close to zero as a few lines of code can get, but definitely not literally zero" :)

@Fishrock123
Copy link
Contributor

Possible options: http://www.thesaurus.com/browse/terminate

@Fishrock123
Copy link
Contributor

I'm personally for adjourn.

@ghost
Copy link

ghost commented Nov 10, 2015

What if you change the word but not the people thoughts?

@cjihrig
Copy link
Contributor

cjihrig commented Nov 10, 2015

I'm -1 for making the change at all. However, if a change is made, I'd like to use a term like voluntary or intentional (as opposed to just a synonym for suicide) to show that the process was intentionally killed.

@gregpardo
Copy link

Not a contributor, but -1 as a node user. This renaming potentially offensive terms is getting out of hand in the open source community. These unix terms have been around for years and they are fine. Who is even getting offended?

@mikeal
Copy link
Contributor

mikeal commented Nov 10, 2015

These unix terms have been around for years and they are fine.

Can someone show me where suicide is a commonly used unix term? I can't find any evidence of this yet it is continually asserted as being true.

@evanlucas
Copy link
Contributor

+1 for replacing. How about voluntaryExit since the docs state that it is set when the worker voluntarily exists

@joshmanders
Copy link

While I agree with kill being a universal unix term, suicide is worthy of a change. I've never seen anything in unix using that term.

@max-mapper
Copy link
Contributor

I support changing it. There is no downside to changing it. Mental health is a very real issue for many people and small notions can make a difference. People commenting about "who is getting offended" please ask yourself why you are spending energy arguing in this thread?

@rennat
Copy link

rennat commented Nov 10, 2015

± for replacing. If we do replace, suggested replacement: seppuku

@chrisdickinson
Copy link
Contributor

+1 for replacing. Asking for people to step up and say "I am the one getting offended by this," with a tacit, unstated "so we can talk you out of it", is not productive. If it's being broached here the person broaching it probably has a good reason in mind, and it's a sensitive topic so they may not want to share it.

If you don't personally have negative connotations with that word — if it's just another verb to you — that's totally fine and OK; but it means a lot to others, and we should be mindful of that. We lose nothing in changing it (a semver major, maybe, which we've long since figured out how to pull off in a repeatable fashion), and make other folks lives slightly easier. Isn't that what OSS is supposed to be all about?

@ashleygwilliams
Copy link
Contributor

💯 on the change. i like suggestions that involve including whether it is voluntary or not, as well.

understood that it needs to be an alias for now for backwards compatibility, but let's mention in the docs that the terms are deprecated so there's a projected road-map for eventually eliminating them. (happy to help with this communication/documentation)

also, i'd like to say that the people who have taken this thread as an opportunity to make jokes should take a serious look at their motivations for doing so. does the suggestion that this change be made make you uncomfortable? this is not a joke, and is a worthy issue. i don't believe those comments have a place here.

@btford
Copy link

btford commented Nov 10, 2015

+1 for the change, with alias to avoid breaking backwards compat.

@Fishrock123
Copy link
Contributor

I like voluntaryExit. It's more descriptive, and also not callous like the current term is. Terming anything suicide is quite frankly disgusting.

Being welcoming (and understandable) in the words our code uses can only do good in growing the node community. We can keep aliases when necessary, though our release process should better enable the deprecation of older APIs over time.

@mcdonnelldean
Copy link

+1 from me to change it. There are far more important things to be doing instead of bikeshedding words. If a term makes someone uncomfortable and can be easily aliased I don't see the issue with doing so to make the codebase more accessible. Nobody is losing anything here.

@jasnell
Copy link
Member

jasnell commented Nov 10, 2015

Let's keep things calm on all sides of the discussion. The majority of folks who have weighed in appear to be in favor of making the change and deprecated the previous term so that's likely what should happen. For the folks who feel that the change shouldn't be made, please just keep in mind that it doesn't hurt any to be sensitive and existing code should not break with appropriate aliasing/deprecation.

@lukewestby
Copy link

👍 for voluntaryExit. it both removes the emotional impact on those affected by suicide and is more clear and descriptive of what actually happened from a technical standpoint.

@MylesBorins
Copy link
Contributor Author

At this point I think we have a very wide variety of opinions and a PR #3743 is already in.

I'd like to suggest locking the thread

@ashleygwilliams
Copy link
Contributor

+1 to locking, @thealphanerd

@nebrius
Copy link
Contributor

nebrius commented Nov 10, 2015

+1 to locking as well @thealphanerd

@nodejs nodejs locked and limited conversation to collaborators Nov 10, 2015
@indutny
Copy link
Member

indutny commented Nov 10, 2015

I think it is never a bad idea to remove offensive words from the API.

Ideally it would be great if we could catch such naming mistakes earlier, before they will go into release. Is there anyone who could volunteer for this? Maybe we should have a WG for this?

@indutny
Copy link
Member

indutny commented Nov 10, 2015

Oh, @isaacs, you locked it just when I posted a question.

@MylesBorins
Copy link
Contributor Author

@indutny perhaps we should start a thread about this in @nodejs/diversity?

@indutny
Copy link
Member

indutny commented Nov 10, 2015

@thealphanerd makes sense!

@mikeal
Copy link
Contributor

mikeal commented Nov 10, 2015

@indutny I doubt that this verb would have made it past the review process we have in place already. This API was added when things were very different and I don't think we're at risk of adding new APIs that lack this sensitivity.

@isaacs
Copy link
Contributor

isaacs commented Nov 10, 2015

@indutny @thealphanerd nodejs/inclusivity#9

@mikeal feel free to point to those review processes if you think they're adequate moving forward.

@mikeal
Copy link
Contributor

mikeal commented Nov 10, 2015

@isaacs improvements to the existing cluster API triggered a review which lead to this issue in the first place, so #3720 would be an example :)

@Qard
Copy link
Member

Qard commented Nov 10, 2015

I'll just say that, as someone who has merged a few PRs to node before, I was not instructed on any sort of terminology review process when invited to be a collaborator. Not sure what the experience was like for later rounds of new collaborators.

@jasnell
Copy link
Member

jasnell commented Nov 10, 2015

Well, as the person who raised the initial concern (#3720 (comment)) I'm happy to see this being addressed. I don't think we need any kind of formal process around this but as a best practice we should, in general, try to be incrementally better at this kind of stuff.

@mikeal
Copy link
Contributor

mikeal commented Nov 10, 2015

@jasnell you wrote the dev policy and the contributing docs, is there an obvious place to note something like this in the contribution guidelines?

@Fishrock123
Copy link
Contributor

I'll just say that, as someone who has merged a few PRs to node before, I was not instructed on any sort of terminology review process when invited to be a collaborator. Not sure what the experience was like for later rounds of new collaborators

It's a little better now, but not that specific.

See: https://github.com/nodejs/node/pull/3726/files#diff-c00e7ffdc40896f8ff6567c07f9eb43cR15 (And other spots), but it could be better and clearer still. Keep in mind this is only a rough draft me and @chrisdickinson have been working off of. We need a place for high-level project values, including this one.

@Qard
Copy link
Member

Qard commented Nov 10, 2015

Ah, cool. That looks like a big improvement from where I came in. 😄

I definitely would like to see a core-values.md or something like that too.

@jasnell
Copy link
Member

jasnell commented Nov 10, 2015

@mikeal ... I can take a look. One thing to note, however, is that while we have documented practices for contributing we don't actually have documented practices for reviewing those contributions. What are the things to look out for? What are the best practices for mentoring new collaborators? @Fishrock123 recently just started working on onboarding docs (#3726), perhaps that would be the best place to cover this?

@mikeal
Copy link
Contributor

mikeal commented Nov 10, 2015

@jasnell I think that the process for reviewing is essentially making sure the contribution passes our contributing guidelines. TBH, these documents are so large it's not reasonable to expect every contributor to understand every inch of them, that's why we have a review culture that brings PRs inline with the contribution policy and through the review process is how most people learn the guidelines.

@jasnell
Copy link
Member

jasnell commented Nov 10, 2015

@mikeal ... yep, we should work on simplifying them. However, much of this can be helped by setting clear expectations during the on-boarding process and by encouraging the existing base of collaborators to lead by example.

@jasnell
Copy link
Member

jasnell commented Nov 10, 2015

I should say, however, that the process for reviewing contributions does go beyond just making sure the contribution guidelines are followed. There's an art to it. If I see a PR that includes a change that I disagree with, I can choose to respond in a number of ways: (a) I could choose what I believe should come to be known as The Torvalds Method, (b) I could use sarcasm to ridicule the basis for the change, (c) or I can take the time to present a logical argument against the change without resorting to either of the previous two options. The choice of response has a definite impact. Further, for any proposed change that I may disagree with, I have to ask myself if there's really any harm in making the change -- even if I think the change is pointless or just "churn". How you review a PR is just as important as ensuring that what is being reviewed passes muster.

@Fishrock123
Copy link
Contributor

@jasnell can we move that to the onboarding PR? :)

@mikeal
Copy link
Contributor

mikeal commented Nov 10, 2015

or I can take the time to present a logical argument against the change without resorting to either of the previous two options

There's also the case where a reviewer finds the contribution should "obviously" not make it in and uses that as the basis for their review, which is almost never true. Maybe something like "the intention of every review is to educate the contributor through the review process"

@ChALkeR
Copy link
Member

ChALkeR commented Nov 11, 2015

Also just noting here that my local goverment had GitHub banned over IP at ISPs level for several days for the file suicide.txt in some repo (well, actually for it's contents).

Things like that could be overdone. To anyone sure that we need a deprecation/removal and one more potential ecosystem breakage over that — please, keep that in mind.

Even soft deprecation means imposing some amount of work over the ecosystem. Is a non-technical reason a good enough excuse for that?


Edit: to anyone who read the first part incorrectly or did not understand why I mentioned it — that was an example of an unreasonable reaction to solve a minor issue (suicide.txt file on a GitHub), causing a bigger one (GitHub outage in a country).

Also, I'm not calling the «suicide» issue negligible, but I hope that it does not worth an immediate ecosystem breakage.

@ChALkeR
Copy link
Member

ChALkeR commented Nov 11, 2015

@isaacs please be consistent with your judgments. #1854 (comment)

I'm not trying to upset anyone, but a lack of strict policy on deprecating things, varying procedure of doing so, and the potential ecosystem breakage bothers me.

@sam-github
Copy link
Contributor

Folks, voluntaryExit is not acceptable as a replacement for suicide:

var cluster = require('cluster');

if (cluster.isMaster) {
  cluster.fork()
    .on('exit', function() {
      // this.suicide is false here!
    });
} else {
  process.exit(0);
}

The above code is an example of what most humans would regard as a voluntary exit. However, in it the .suicide (edit: aka voluntaryExit) would be false! Ouch.

I'll kick off this off again with the suggestion of expectedExit.

@ChALkeR
Copy link
Member

ChALkeR commented Nov 27, 2015

@sam-github Could you also mention that in #3743? That's where the current discussion is located.

@evanlucas
Copy link
Contributor

Closed via 4f619bd

Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cluster Issues and PRs related to the cluster subsystem.
Projects
None yet
Development

No branches or pull requests