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

Branch Protection #39

Closed
ChrisJefferson opened this issue Nov 4, 2023 · 2 comments
Closed

Branch Protection #39

ChrisJefferson opened this issue Nov 4, 2023 · 2 comments

Comments

@ChrisJefferson
Copy link
Contributor

I suggest we enable "branch protection" for main -- basically this stops people (and people definately includes me) from accidentally pushing to main. I've done this in other projects accidentally.

This actually comes with a whole bunch of settings, here is the list, and my experience of them with another fairly large multi-person project ( GAP at gap-system/gap ), but I'm very happy for people to disagree, and also we could try changing settings later.

  • Require a pull request -- Yes, everything should go via PR.
  • Requre status checks to pass -- No, seems like a good idea, but then some status check breaks for a while and merging is annoying, just trust people to not press merge? We could maybe enable this later, when the various checks seem to be stable and usually passing.
  • Conversation resolution before merging -- No, if someone makes some small comments, it's annoying (in my experience) to have to go press "resolved, resolved, resolved" before merging -- trust people again to read comments before merging.
  • Require signed commits -- No, just annoying
  • Require linear history -- ?, probably No?, this would require always 'rebasing' instead of 'merging'. Personally I think this, but it would require people be happy with rebasing.
  • Require merge queue -- No, dunno what this does
  • Lock branch -- Yes, this stops people pushing (the main point)
  • Do not allow bypassing the above setting -- Yes, this stops admin from being stupid :) If they ever need to do it for some reason, they can just pop into the settings and temporarily turn off protect.
@ozgurakgun
Copy link
Contributor

Done. Additional options regarding approvals appeared when I checked "Require a PR before merging". I didn't check any of these as I don't want to require approvals at this stage. Let me know if you think we should make any other changes to this too!

@ozgurakgun
Copy link
Contributor

Update: I think "lock branch" really makes the branch read-only (won't allow merging PRs). I unchecked this.

I am not sure if this is the behaviour but I went to merge #34 with "lock branch" checked and it wouldn't allow me. Unchecked now and it does.

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

2 participants