util: move custom inspection function to EOL

[Trott]

Move the use of custom inspection function to End-of-Life.

Ref: #15549

Checklist

• documentation is changed or added
• commit message follows commit guidelines

[Trott]

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite/664/

[Trott]

Since this is semver-major, it's going to need some @nodejs/tsc reviews. (Fortunately, it is a one-line change.)

[mcollina]

LGTM

[TimothyGu]

The commit message should be changed to

"util: move custom .inspect function to EOL"

We still support custom inspection through the inspect symbol.

[TimothyGu]

Also, does this require any code changes?

[addaleax]

Also, does this require any code changes?

Yes, I think we want to remove the corresponding code from lib/util.js.

[Trott]

Also, does this require any code changes?

Yes, I think we want to remove the corresponding code from lib/util.js.

My initial reading of the documentation left me with the impression that the process was to move the API to End-of-Life and then (optionally!) remove the API itself in a subsequent release. Reviewing a second time, it appears that perhaps we can remove the API at the same time as moving it to End-of-Life, but it does not appear to be required. It can be removed at a later date (unless I'm missing something?).

[Trott]

My initial reading of the documentation left me with the impression that the process was to move the API to End-of-Life and then (optionally!) remove the API itself in a subsequent release.

/ping @jasnell because I believe he was the original author of the deprecation policy. Was the intention for things to be removed at the same time as moving to End-of-Life? Or to potentially be removed subsequently? Or both situations were expected to arise?

[addaleax]

@Trott I guess just wouldn’t know what “End-of-Life” would mean, if not a full removal of the API? At least it seems odd to keep a runtime deprecation but not call it that way in the docs…?

[BridgeAR]

This has to come with the code change.

[Trott]

@Trott I guess just wouldn’t know what “End-of-Life” would mean, if not a full removal of the API?

I can see the thinking there for sure, but according to our documentation:

End-of-Life deprecation is used to identify code that either has been removed or will soon be removed from Node.js.

[addaleax]

@Trott yeah … not sure what to think of that in the docs :/

Like, in particular I don’t quite understand what benefit users get from having something listed as EoL over having it listed as runtime-deprecated? Is it to say that we’re committed to removing it in the next major version? In that case we might want to just say that explicitly…?

[Trott]

This has to come with the code change.

I can certainly see how one might assume that, and perhaps we should changing our process to requiring that removal happen at time of labeling EOL, but right now, our documentation says:

An End-of-Life deprecation is used to identify code that either has been removed or will soon be removed from Node.js.

So removal is not necessary. (It should happen soon, though.)

[Trott]

@Trott yeah … not sure what to think of that in the docs :/

Maybe we should consider changing it to be more in line with the understandable intuition you and @BridgeAR have about it. It will need to be changed in both deprecations.md and COLLABORATOR_GUIDE.md. We'll also need to go through existing EOL deprecations and make sure they are all in fact removed.

[joyeecheung]

Can you add a TODO where the DEP0079 warning is emitted? Just in case it is forgotten..

[jasnell]

The idea is that End-of-life takes the thing out from under the semver-rules so that it can either be removed or more freely refactored, whichever the case may be. Typically, it means removal and, yes, it can be removed the same time as it hits End-of-life

[BridgeAR]

I opened #20722 for completely removing custom inspection.

This should be fine to land but I would prefer to land it right before ##20722 and not any earlier. So I just added the blocked label.

I still think we should rewrite our guidelines to recommend to always do the actual breaking change at the same time as the documentation change. Otherwise there is no real point in changing the documentation as far as I can tell.

[Trott]

@BridgeAR Since this PR is a one-line trivial change, feel free to add that to your PR (if you haven't already) and close this one. (Or if you're a stickler for preserving author credit, you can cherry-pick my commit into your PR. But I don't think that's necessary in this case.)

[addaleax]

Or if you're a stickler for preserving author credit

Btw, something I learned today: https://help.github.com/articles/creating-a-commit-with-multiple-authors/ :)

[BridgeAR]

Superseded by #20722