module: deprecate Module._debug#13948
Conversation
|
This one has been there for so long we might need to do a deprecation cycle on it. Marking semver-major defensively. Ping @nodejs/ctc |
|
(to clarify, I would highly doubt that it's being used, but it pays to be safe) |
|
This needs some more reviews from @nodejs/ctc |
|
@jasnell I guess it is safe to land this as is? |
|
Perhaps we should ping @ChALkeR and ask him to please please please do a quick ecosystem search, just to be safe. We should also do a CITGM run just in case. It also needs CI |
evanlucas
left a comment
There was a problem hiding this comment.
Can we deprecate this like we normally do? It shouldn't cost anything. If it does, we will get complaints which will show it is being used.
|
It wouldn't hurt to deprecate it. |
|
Yep, I'm good with a regular deprecation. |
|
Thanks all. I will change to deprecate it first. |
e35b098 to
26c981e
Compare
|
Hi everyone, I updated this PR with deprecation. |
doc/api/deprecations.md
Outdated
There was a problem hiding this comment.
Would you mind escaping the underscore?
doc/api/deprecations.md
Outdated
There was a problem hiding this comment.
This should actually be DEP00XX until it lands. The person landing needs to assign the specific code.
There was a problem hiding this comment.
And the
Module._debug = util.deprecate(debug, 'Module._debug is deprecated.',
'DEP0076');should be DEP00XX also?
There was a problem hiding this comment.
Yeah, all of the DEP00.. identifiers.
The _debug of Module is undocumented and it useless here.
|
Hi all, I have updated this PR. |
|
@evanlucas the PR was updated with your suggestion. PTAL |
|
Our deprecation guidelines don't cover this. So I'll just ask here. Do we allow an undocumented API to be runtime deprecated directly without documenting and deprecating first? |
|
@thefourtheye that is a good point. I guess we should go ahead and improve the deprecation guidelines. I personally do not feel like it is the right thing to do to document a so far undocumented feature with the goal of actually removing it at some point as that is somewhat counterproductive. |
|
Ping @evanlucas |
|
I think it's likely safe to dismiss @evanlucas' objection since this was changed to a deprecation as he requested. Evan is on vacation right now so may not be able to rereview soon |
Was changed to a deprecation as requested
Yes, so long as |
|
Landed in 9d7574e |
The _debug of Module is undocumented and it useless here. PR-URL: #13948 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The _debug of Module is undocumented and it useless here. PR-URL: nodejs/node#13948 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The _debug of Module is undocumented and it useless here.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
module