Node: Should issues creating churn on 100s of files require backports to land?

[MylesBorins]

I was just reviewing https://github.com/nodejs/node/pull/14162 which created a new lint rule and touched 294 with a very subtle style churn. I have no problems with this type of change landing, but I think that we may want to consider pre-opening backport PRs as part of the sign off requirements.

The potential for running into conflicts mid audit becomes quite large when touching a large percentage of the tests at once like this.

[joyeecheung]

SGTM, although we could lower the number 100 if necessary. Maybe a new label like needs-pre-backport so the person landing PRs will be reminded to double-check?

[MylesBorins]

another commit touching > 100 files

https://github.com/nodejs/node/pull/13757

This seems to be a pattern in the last couple months that we had not done in the past

@nodejs/tsc how would you suggest going from here? Should we add this to the agenda?

@nodejs/collaborators would enjoy some more insight on this

[Trott]

I love this idea.

[jasnell]

I'm -0 on requiring a backport PR in order for the original to land. I'd be more inclined to not backport these types of changes (and yes, I fully realize that not doing so creates it's own backporting burden)... but I would argue that these kinds of broad brush changes do not necessarily need to be backported at all.

[joyeecheung]

@jasnell One problem is, if they have touched enough files then not backporting them will result in more conflicts later, because new PRs will come to depend on those changes, especially those touching a lot of tests

[jasnell]

Yeah, I've been through that pain first hand, which is why I'm only -0. I don't think this should be a hard rule, but a useful recommendation. There are just going to be cases where backporting them is not necessary

[mcollina]

I think this is a great idea. It should simplify the work of the release team.

[cjihrig]

I think this is a good idea.

[BridgeAR]

I like the idea. There might be some difficulties with that as well but I think we should give it a try.

[gibfahn]

I like the idea, and I agree that if it touches enough files then backporting is necessary for the sake of the backporting team.

My worry would be that this will lead to us backporting things straight away. How would this work if we wanted to leave the PR to bake in current for a release or two?

[vsemozhetbyt]

I recall that, for some of my big-churn PRs, LTS maintainers ask me to promise to do a backport when the time comes. Maybe we can just formalize such promises? Or this does not guarantee us anything?

[vsemozhetbyt]

We can also ask that all the reviewers with LGTM would promise to help with a backport if the author cannot do it due to some circumstances (+ re-review the backport).

[vsemozhetbyt]

Also, maybe it is worth to mention in the guides that a contributor may need to ask in an issue if some big-churn PR would be accepted (and would have reviewers and backport guarantees) to prevent disappointing and wasting time.

[gibfahn]

I recall that, for some of my big-churn PRs, LTS maintainers ask me to promise to do a backport when the time comes. Maybe we can just formalize such promises? Or this does not guarantee us anything?

We can also ask that all the reviewers with LGTM would promise to help with a backport if the author cannot do it due to some circumstances (+ re-review the backport).

I think we should formalize both of these things for all PRs, to help set expectations. If your PR gets landed and needs a backport, you should be ready to do said backport. Obviously help is available if you're unsure.

[MylesBorins]

I like all of these ideas! Does anyone feel like taking the action item of formalizing some of this in the Contributor guide?

[joyeecheung]

@MylesBorins I think this actually belongs to the collaborator guide?

So far the ideas are:

• If a PR creates churn on > 100 files, a pre-backporting PR should be requsted/required (not sure about which)
  
  
  
  • The person creating that PR does not have to make the promise, reviewers can do that as well
  
  
• Add a label for this request
• Suggest that if one wants to make a PR that would create a lof of churn, discuss on the issue tracker first to avoid wasting their time.

[gibfahn]

@MylesBorins I think this actually belongs to the collaborator guide?

This is for all contributors though, not just collaborators, so maybe Contributing makes more sense.

[joyeecheung]

@gibfahn Yeah. but my instincts was this should be placed where the "semver-major needs two TSC sign-off" is, and turns out that's in the collaborator guide. I guess that makes sense because it's the people landing it should be the most careful.

[MylesBorins]

I'll put something together and submit to the collaborator's guide

[srl295]

Since I create such churn yearly… yes, I'm willing to take on backports for
ICU if needed. Would you like backports at the same time as the main PR
lands for ICU? I ask because ICU 60 will be here in a week or so.