12.x reverts

[MylesBorins]

This includes are revert of #31063 and 65e5a8a as both seemed to have caused regression in v12.16.0

#31752
#31249 (comment)

I believe that 65e5a8a should be able to land with a fix in the next 12.x release. As This includes are revert of #31063 is SemverMinor it would have to wait until April to get out in the next release.

As such I wanted to open this PR to get consensus on landing the reverts before doing so.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/29118/
rerun linux containered: https://ci.nodejs.org/job/node-test-commit-linux-containered/18055/

[MylesBorins]

Landed in f0b2d87...2ea5c91

[addaleax]

@MylesBorins I think it sets a bad precedent that we’re reverting changes on v12.x only when they should also be reverted on master…

#31784 does that partially, and I’ll open a partial revert for 65e5a8a (along the lines that I’ve requested in the v12.16.1 proposal), but doing it on release staging is just bound to get messy and we should really avoid that if we can

[jasnell]

Oy, yeah, when reviewing this I hadn't realized that the issue also affected master and 13.x... the PR text indicated a regression only in 12.x. These really should have been addressed in master first then backported to keep things from getting messy.

[bnoordhuis]

Also, doesn't the 48 hour rule apply to release branches? This was opened and landed on the same day (edit: within 8 hours even.) That doesn't really give people time to comment.

[MylesBorins]

@jasnell the choice to revert was exactly because other lines were also affected. There were no fixes added here, but rather reverting the changes that introduced the regressions before the fixes landed on 13.x and master in order to respect the LTS policy. Even if it "reintroduced" regressions that were fixed by the change, that behavior is at least what is currently expected on LTS.

@bnoordhuis to the best of my knowledge, and how I've personally been working for years, the 48 hour rule does not apply to release branches. The release branches are maintained by the release / LTS team and have their own rules that govern them. In the case of this reversion PR there was an attempt to get a fix out within 24 hours of recognizing that there were regressions on 12.x. If the 48 hour rule applied it wouldn't be possible to do quick releases and imho there would be significant friction introduced to the general process. The reason to open this PR is for quick sign off as well as auditability.

FWIW, if you would like to discuss changes to current process or think it would be beneficial to revisit the existing process I think it is totally worth doing so. Opening an issue in the release repo would be a good way to move forward.