Skip to content
This repository has been archived by the owner on Mar 7, 2023. It is now read-only.

Suggest enabling branch protection for this repository #14

Closed
alerque opened this issue Oct 7, 2020 · 2 comments
Closed

Suggest enabling branch protection for this repository #14

alerque opened this issue Oct 7, 2020 · 2 comments
Labels
management Not a discussion or task

Comments

@alerque
Copy link
Collaborator

alerque commented Oct 7, 2020

I would like to suggest that we enable branch protection for the main branch of this repository such that nobody is able to push directly to main but all commits must flow through a PR based workflow. This does add a small amount of 'overhead' to some operations. For example I saw a punctuation mistake just now and was tempted to push a fix through directly ... but given the nature of this group I believe it would be better if we kept everything more visible. Pushing directly to main without a PR step has one major downside: invisibility. Yes there is a paper trail and the change is there for all to see — but only if you look for it. Forcing a PR workflow adds notifications into the loop. People that choose to "watch" this repository will get either notifications on-site or by email (per their own configuration). Perhaps nobody cares, but I would appreciate this transparency and I think others might too.

  1. As such I propose adding branch protection, disabling force push, and requiring a PR workflow to merge to main including for administrators.

  2. Secondarily (and agreeing to the first part of this does not imply agreeing to this part) I also suggest that the PR workflow require 1 approving review for merger. This is much less important in my view than the first step, but think we probably have enough eyes and involvement here for this to be a very low overhead way to make sure 2 eyeballs actually get on any change that goes through.

Do note that this is not a security measure as the current administrators still have access to unilaterally disable these 'protections', they just have to willfully change the settings or access them dodging big red buttons that say "Yes use my admin privileges to force this operation". This proposal just firmly nudges everybody towards a more public workflow.

Please voice support or objections to these two proposals separately as the first would still allow repository administrators to unilaterally push changes through on as fast a timeline as they wanted, it would just mean interested parties got notified; the second would potentially slow the process down by requiring a second person to sign off on everything.

@lianghai
Copy link
Contributor

lianghai commented Oct 8, 2020

I agree with both proposals, and I think we can proceed without the CG’s collective approval.

Please go ahead and implement both, and be prepared to brief the Meeting 4 attendees on this. We can revert the configuration later if there’s objection at Meeting 4.

@lianghai lianghai added the management Not a discussion or task label Oct 8, 2020
@alerque
Copy link
Collaborator Author

alerque commented Oct 8, 2020

I implemented both counts. The required review count is currently set at 1.

I'll be prepared to brief on this at the meeting (which I'll try not to miss this time!).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
management Not a discussion or task
Projects
None yet
Development

No branches or pull requests

2 participants