Codecov Report

Merging #420 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #420   +/-   ##
=======================================
  Coverage   77.13%   77.13%           
=======================================
  Files          21       21           
  Lines        1496     1496           
=======================================
  Hits         1154     1154           
  Misses        342      342           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7dc544d...a3c6eb7. Read the comment docs.

It would be better if this was part of the git node land <pr> workflow to stop DEP0XXX landing in the first place. Typically we'd fix those up in master before backporting (because I think we want the numbers to be consistent across master/release lines). I guess we could fix these at release time as long as the change is merged upstream afterwards (hopefully without conflicts).

@richardlau good point - i added this since it came up yesterday during the v14 release with this PR, but a second pass of the releases guide shows:

This assignment should occur when the PR is landed, but a check will be made when the release build is run.

I'll migrate this logic into the land logic!

@richardlau good point - i added this since it came up yesterday during the v14 release with this PR, but a second pass of the releases guide shows:

This assignment should occur when the PR is landed, but a check will be made when the release build is run.

I'll migrate this logic into the land logic!

If you want to also codify the check during release (in addition to modifying the land logic) I'd have no objection 😁.

@targos i'd love to be able to get this in before next week's release if you have some time to peek at it 🙇‍♀️

I'm afraid this cannot be easily automated, because there may be multiple DEP0XXX placeholders for different deprecations introduced in different PRs. nodejs/node#33294 was special because it introduced more than one deprecation in a single PR.
I prefer an approach where we prevent placeholders to land at all (something nodejs/node#33433 is attempting to do).

@targos in that case - shall i just adapt this PR to check for unmarked DEP0XXX tags and not try to fix them?

What about a compromise : fix it if there is only one placeholder in deprecations.md, otherwise pause and ask to fix manually?

@targos i've thought about this a bit more - i think it would still make sense to automate multiple in the landing stage? I can't think of a case where there will be two conflicting deprecations introduced in the same PR 🤔

In the release stage it makes sense only to automate if there's one though, i agree! I bumped some related changes and if there's an edge case i'm missing i'll remove the multiple functionality from landing!