doc: add documentation for killed property of ChildProcess instance

[Trott]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

doc child_process

[refack]

Do we really want to go there? Can we skip straight to child.terminated? (deprecate killed and alias to terminated)

[jasnell]

I agree, it may be a good opportunity to improve the naming here.

[refack]

looking at the code

if(this._handle){varerr=this._handle.kill(signal);if(err===0){/* Success. */this.killed=true;returntrue;}

So child.signalled will probably even be more correct, since it will be true for child.kill('SIGUSR1')

[Trott]

Do we really want to go there? Can we skip straight to child.terminated? (deprecate killed and alias to terminated)

TL;DR: I'd be more inclined to change child in the docs to subprocess than trying to alias killed.

A few things to bear in mind:

• child.kill() is already documented and isn't going anywhere. Is it worth the effort to avoid child.killed when child.kill() is pretty much unavoidable and in the docs? (Rhetorical question, but I also don't know the answer. Maybe it's "yes" but I'm not sure.)

• If we're going to change it up, IMO it's a more user-friendly change if we change it to subprocess.kill() and subprocess.killed. At least then users don't have to alter their code that works just fine right now.

• Even if we deprecate .killed and add an alias, we still need to document .killed. It's in use in the ecosystem.

I'm happy to add a second commit to this PR that changes child to subprocess before the commit that then documents killed but I'd want to make sure there's consensus that this would be a good way to go.

[refack]

PR that changes child to subprocess

Sounds like a good solution.

[cjihrig]

+1 to changing to subprocess

[Trott]

One thing that changing to subprocess will do is break anchor links to our docs like https://nodejs.org/dist/latest-v8.x/docs/api/child_process.html#child_process_child_kill_signal. For that reason, I think it should be semver-major so that the change only lands when the URL is changing anyway because latest-v8.x is being bumped to latest-v9.x.

[jasnell]

The anchor link breaks can be addressed by including <a id="..."></a> markup in the doc with the old generated ids.

[Trott]

The anchor link breaks can be addressed by including markup in the doc with the old generated ids.

I'm OK with that approach. I do have a slight preference for landing in Node.js 9.x and skipping the introduction of raw HTML into the markdown files though. This property has never been documented. It can wait another two months.

[Trott]

Oooh, maybe we can only add the HTML markup in backports so they never show up in master.

[Trott]

OK, added the change to subprocess as a first commit. Doc subprocess.killed in the next commit. Instead of semver-major, I'm adding backport-requested for 6.x and 4.x. Once this goes through review and lands, I (or someone else if someone's motivated) can open backport commits that will add in the anchors so that existing URLs (from external sources to the 6.x and 4.x docs) don't break.

[Trott]

@cjihrig@lpinca Lots of changes since your approvals. You might want to take another look?

[lpinca]

Missing opening backtick.

[Trott]

@lpinca Oops, fixed, thanks!

[cjihrig]

LGTM with missing backticks added.

[refack]

LGTM % backtick fix

[fastman]

@Trott

PR that changes child to subprocess

Great! I've tried to suggest such change here #14444 (comment) but no one liked the idea then :/