Repository navigation
Remove 48 hour waiting period for npm updates opened by members of the npm team #35954
Description
Hey all,
As the landing of npm updated tend to be RSLGTM I was curious if folks would be open to removing the need for the 48 hour rule when those PRs have been opened by members of @nodejs/npm and have at least 2 sign offs.
Thoughts?
Activity
gireeshpunathil commentedon Nov 4, 2020
@MylesBorins -
MylesBorins commentedon Nov 4, 2020
As we are doing a dependency update we wouldn't generally make changes, if folks saw issues they should patch them upstream.
It is possible we might want to make this policy for all dependencies not just npm, at the very least dependencies being updated by their maintainers.
npm has a rather rapid iteration process right now for 7.x. We have been fairly consistently fast tracking. 4 out of the last 5 npm updates were fast tracked. We can continue to get a fast track through the normal process but it seemed reasonable to maybe get a one time exception
gireeshpunathil commentedon Nov 4, 2020
+1 to the
prosalproposalrichardlau commentedon Nov 4, 2020
@MylesBorins I'd lean more to requiring two sign offs and at least one being from a member of @nodejs/npm. I personally don't see who opened the PR should factor into it as long as they used the update script that you recently added (#35822).
The tooling (node-core-utils/commit queue) won't understand anything outside of the existing fast track process though unless changed. (And if we do change the tooling I'd prefer a more dep-neutral process change rather than one just for npm (but 👍 for npm maintainers being active here).)
mhdawson commentedon Nov 4, 2020
Although there has been a fair amount of fast tracking recently, I think that is just that the process is working as intended to deal with special cases. I see npm having more than the regular amount of changes being a special case versus something that we should change the policy for long term.
mmarchini commentedon Nov 4, 2020
We still need to make the changes to the wait time that we discussed in TSC meetings recently (reduce to 24h with two approvals from codeowners being enough), which I think would help here? Personally I'm fine with it for npm and maybe others, not sure I'd say the same for V8 (although maybe I'm being unnecessarily extra cautious).
In the meantime, I think it's totally fine to fast-track all npm upgrades if they are open by npm team members and only affect npm-related files.
targos commentedon Nov 4, 2020
It's extremely unlikely that a V8 update PR can be ready and landed in less than 48 hours anyway 😄
MylesBorins commentedon Nov 4, 2020
We can definitely keep following the process in place, generally with 2 sign offs we can get 2 fast tracks... although members of the npm team have been holding off on +1 the fast track as to not have a conflict of interest.
I guess the thought here was more that it doesn't need to be manually reviewed if opened by a member of the team. I think having the sign off needing to be from one of the maintainers is a good middle ground.
Making this a generic policy for dependencies that are maintained by collaborators seems like a good direction to take it.
mmarchini commentedon Nov 4, 2020
IMO it's not a problem if npm folks +1 the fast track request.
Trott commentedon Nov 6, 2020
Yeah, seriously, I'd rather have the domain experts weigh in on the fast-tracking than to rely on my "oh npm update? rubber-stamp LGTM!" judgment.
gengjiawen commentedon Nov 6, 2020
+1 from me.
gengjiawen commentedon Nov 6, 2020
But I think our tools need to add a function to auto add a label (
opened-by-npm-core-devmaybe) if PR opened by members of @nodejs/npmBridgeAR commentedon Mar 19, 2021
The conclusion here seems that fast tracking is the way to go to overcome the regular 48 hours.
Closing as resolved.