build: fast-track npm PRs and dont-land them on LTS #38885
Conversation
github-actions
bot
added
the
meta
Was the intent to add the fast-track label? |
Yes 😅 |
.github/label-pr-config.yml
Outdated
| @@ -78,6 +78,7 @@ subSystemLabels: | |||
| /^deps\/v8\/tools\/gen-postmortem-metadata\.py/: V8 Engine, post-mortem | |||
| /^deps\/v8\//: V8 Engine | |||
| /^deps\/uvwasi\//: wasi | |||
| /^deps\/npm\//: npm, fast-track, dont-land-on-v14.x, dont-land-on-v12.x | |||
There was a problem hiding this comment.
I'm generally -1 on automatically marking dependency PRs as fast-track.
There was a problem hiding this comment.
I'd like to defer to @ruyadorno or someone else from the npm team. I added it because all recent npm PRs were fast-tracked.
There was a problem hiding this comment.
We always fast-track every npm update. We had previously asked about maybe carving out something in the process to allow us to land stuff without a fast track.
Pragmatically... the npm team will always ask for a fast-track on every dep update... and with the update now being automated we will always have at least 2 collaborators to approve the fast-track on hand.
Following the process to be consistent makes sense, but perhaps it is worth again revising the approval process for dependencies that we as a project vendor (and hopefully automate the vendoring)
There was a problem hiding this comment.
thanks @targos! as @MylesBorins contextualized already, in practice we have been always asking for a fast-track on every npm update PR and it would be appreciated to just automate that manual work that is already there today.
That said, another challenge we have currently is that not all people in the @nodejs/npm team seem to have access to add/remove labels in the Node.js repo, so alternatively if we're not landing this change here I would like to see an alternative to make that workflow improved for the rest of the team.
There was a problem hiding this comment.
+1 on @ruyadorno's comments/suggestions - personally prefer that automated label route though
There was a problem hiding this comment.
I won't block if that's what is consistent.
There was a problem hiding this comment.
@jasnell there is the alternative of not requiring the fast-track to land an npm update with 2 reviews 😉
|
@targos This needs rebase. |
|
Landed in 6fe89a1 |
PR-URL: #38885 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
PR-URL: #38885 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
@nodejs/npm