Node: Remove 48 hour waiting period for npm updates opened by members of the npm team

[MylesBorins]

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?

[gireeshpunathil]

@MylesBorins -

• is there a substantial difference between these PRs and regular PRs?
• is there a known issue that we are trying to solve by doing this?

[MylesBorins]

is there a substantial difference between these PRs and regular PRs

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.

is there a known issue that we are trying to solve by doing this?

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]

+1 to the prosal proposal

[richardlau]

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?

@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 (https://github.com/nodejs/node/pull/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]

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]

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]

not sure I'd say the same for V8

It's extremely unlikely that a V8 update PR can be ready and landed in less than 48 hours anyway 😄

[MylesBorins]

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 personally don't see who opened the PR should factor into it as long as they used the update script that you recently added

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]

IMO it's not a problem if npm folks +1 the fast track request.

[Trott]

IMO it's not a problem if npm folks +1 the fast track request.

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]

+1 from me.

[gengjiawen]

But I think our tools need to add a function to auto add a label (opened-by-npm-core-dev maybe) if PR opened by members of @nodejs/npm