Branch protection rules

letsgamedev / Suffragium

A game developed in a democratic cycle.

GNU Affero General Public License v3.0

51 stars 21 forks source link

Branch protection rules #44

Closed Numenter closed 2 years ago

Numenter commented 2 years ago

We should add Branch protection rules, to prevent future faulty PR merges!

You find the Branch protection rules under Repo > Settings > Branches > Add rule.

Which rules should be added?

[x] Require a pull request before merging
- [x] Require approvals
- Required number of approvals before merging: 2
- [x] Dismiss stale pull request approvals when new commits are pushed
- [ ] Require review from Code Owners
[x] Require status checks to pass before merging
- [ ] Require branches to be up to date before merging
[x] Require conversation resolution before merging
[ ] Require signed commits
[ ] Require linear history
[ ] Require deployments to succeed before merging
[x] Include administrators

Numenter commented 2 years ago

I thing, those rules are nice to have.

Require a pull request before merging Require approvals = 2 Dismiss stale pull request approvals when new commits are pushed
Require conversation resolution before merging
Include administrators

BjoernAkAManf commented 2 years ago

Maybe require approvals from previous contributors rather than admins. Also need to update guidelines simuntaneously. Could be helpful to have something like #20 where we keep track of those decisions.

in any case we should not say maybe to rules, but rather say we want those rules. If one disagrees, we can talk about that in particular (e.g. with the critique i said above)

Numenter commented 2 years ago

Maybe require approvals from previous contributors rather than admins

Where does it say this. I don't see it.

I will change the issue structure.

BjoernAkAManf commented 2 years ago

Im refering to include admins. I dont believe thats nwcessary but rathee we need the approval of atleast one contributor.

Github has no settings for that though as far as i know.

Numenter commented 2 years ago

"Include administrators" is the most important rule form all of them. We are trying to protect for faulty PR merges. Who does the merges for us? The all mighty administrator.

Require a pull request before merging just require a approval. It don't have to be someone with rights, right?