Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanding access to landing on staging #388

Closed
MylesBorins opened this issue Nov 18, 2018 · 13 comments
Closed

Expanding access to landing on staging #388

MylesBorins opened this issue Nov 18, 2018 · 13 comments

Comments

@MylesBorins
Copy link
Contributor

Currently in practice we have only allowed changes on staging branches to be landed by members of @nodejs/backporters

The policy was not well documented, which is being rectified in nodejs/node#24465 and #387

Is this policy what we want to be doing? Should we expand the circle of people who can land things?

Traditionally we had limited access to ensure that commits on those branches were ones that had been audited by @nodejs/backporters and were ready for a release. Expanding the circle would require rethinking how we manage that bit. It could simply be requiring sign off from the LTS team on a release.

Thoughts?

@rvagg
Copy link
Member

rvagg commented Nov 19, 2018

I don't mind expanding it and making it much more flexible. We could just move the auditing process to the release proposals for LTS branches or a bit earlier here when planning a release. More hands backporting is probably a good thing and we can always back things out if we decide they don't belong.

@mhdawson
Copy link
Member

I would agree with making it more flexible as well. At very least allowing the sign off would make it possible to share the work a bit more.

@targos
Copy link
Member

targos commented Nov 19, 2018

I'm not sure that being able to land things on the staging branch will encourage more people to actually do the important part of the work (open backport PRs).

@MylesBorins
Copy link
Contributor Author

So I was thinking about this... I think we should perhaps consider enforcing commits to be signed if they are cherry-picked to an LTS staging branch. Thoughts?

@targos
Copy link
Member

targos commented Nov 20, 2018

I'm not against it, but what would be the benefit?
Note that it is the committer that matters for signed commits. If you rebase a branch that has unsigned commits, they will all be signed (with your key) after the rebase.

I think we should at least enforce it for all release branches (see #392).

@MylesBorins
Copy link
Contributor Author

@targos rebasing would remove the history, but we would be able to audit who made what changes prior to a rebase

@targos
Copy link
Member

targos commented Nov 20, 2018

but we would be able to audit who made what changes prior to a rebase

👍

@ofrobots
Copy link

ofrobots commented Feb 1, 2019

Any conclusion on this? I do a lot of backports of upstream V8 fixes. The hard part of the backport process is in getting the upstream changes triaged and modified to work on older node branches. The frustrating part is waiting for someone to land the change. This can take several weeks (e.g nodejs/node#25330). There is nobody at fault here – we are humans, and we have our scalability limits.

IMO, we should be making landing as easy as possible once the backport is done. I can understand the need for someone to audit if the change should be backported in the first place, but in many cases the release/backport team itself had requested a backport. In other cases, all we need a a bit flip saying 'this has been audited' by the release team.

Anything that involves the backport / release team humans to do work to land changes is only going to make things worse. I don't have opinion on the mechanism to do the 'bit-flip' – whether it is signatures on the commit, or a label in the backport PR. I do strongly believe that we are over burdening the humans in the release / backport team and the current process is not scaling.

@mhdawson
Copy link
Member

mhdawson commented Feb 1, 2019

I'd agree that once the backporting team has "approved" a backport then it would be good to be able to get it landed once it is ready. Would it be any easier if it just required an "approval" from a backporting team member in addition to regular approvals?

@BridgeAR
Copy link
Member

It seems like everyone agrees that it would be beneficial to allow backporting PRs by everyone. I personally would like the commits to be signed but I would like that to be the case for all commits from collaborators. We might just make that a separate decision?

I believe the only action item left here is to update some documentation @nodejs/releasers.

@MylesBorins
Copy link
Contributor Author

I think commit signing for LTS branches is also about know who landed what as much as it is about the signature, I don't think this needs to be coupled with a decision upstream.

A next step is definitely drafting some guidance / governance around this

@targos
Copy link
Member

targos commented Apr 29, 2019

I think we can require commit signing for all release branches.

@MylesBorins
Copy link
Contributor Author

Closing this as the discussion has gone stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants