rems-project / cn-tutorial

7 stars 8 forks source link

Turn on branch protection #41

Closed septract closed 1 month ago

septract commented 1 month ago

We should avoid pushing to main, but also retain the ability to do it if strictly necessary. GitHub offers a few tools to control this - see here.

I would propose we set this up as follows:

bcpierce00 commented 1 month ago

I agree this is a reasonable step, with so many people working on the repo now.

We should also find a way to discuss / implement a bit of directory structure rationalization.

On Fri, Jul 12, 2024 at 8:58 PM Mike Dodds @.***> wrote:

We should avoid pushing to main, but also retain the ability to do it if strictly necessary. GitHub offers a few tools to control this - see here https://urldefense.com/v3/__https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches__;!!IBzWLUs!UtjEFLDoPlTaZUclWHummDMA4k55Cso9TiZsVQyvu_GI-6X94Tb9JAO0lUmhWirQ8Stjrxpuppkd9qD_0-cpRWKhjQkJ$ .

I would propose we set this up as follows:

  • Forbid pushing to main, and require all commits to go through PR review
  • Allow admins to circumvent this restriction, with the understanding they will only do so in unusual circumstances.

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/rems-project/cn-tutorial/issues/41__;!!IBzWLUs!UtjEFLDoPlTaZUclWHummDMA4k55Cso9TiZsVQyvu_GI-6X94Tb9JAO0lUmhWirQ8Stjrxpuppkd9qD_0-cpRXeAM56y$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ABVQQCZUBF2XQJB7WEQCDG3ZMB3RTAVCNFSM6AAAAABKZZGFW2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGQYDMNJVGQ4DSMQ__;!!IBzWLUs!UtjEFLDoPlTaZUclWHummDMA4k55Cso9TiZsVQyvu_GI-6X94Tb9JAO0lUmhWirQ8Stjrxpuppkd9qD_0-cpRXM0Vwrx$ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

septract commented 1 month ago

Separate ticket to discuss rationalizing the directory structure: #43

dc-mak commented 1 month ago

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#require-linear-history

Perhaps this (and its pre-reqs) as a minimum to start?

septract commented 1 month ago

Are there counter arguments to just turning on branch protection fully? If we can reach consensus, I'd rather do the right thing first time

bcpierce00 commented 1 month ago

What does "fully" mean here? (I.e., is it more than your original proposal, @Mike Dodds @.***>?)

Message ID: @.***>

septract commented 1 month ago

Fully means my original proposal, as opposed to @dc-mak's suggestion (which I took to be a half-way, but maybe that's an incorrect interpretation?)

dc-mak commented 1 month ago

I'm happy with fully, I propose halfway out of pre-emptive pragmatism rather than preference.

ZippeyKeys12 commented 1 month ago

Are the two not independent? We'd like to require PRs, but we'd also like those PRs to be either squashed or rebased?

septract commented 1 month ago

Requiring linear history and forbidding pushes direct to main are both good and we should do them both. And no-one seems to have raised any objections to this idea.

@cp526 or someone else with privileges, can you turn this on?

cp526 commented 1 month ago

@septract If I got it right, this should be in place now: main has to have linear history, and commits to main have to go via pull requests from branches.

septract commented 1 month ago

Yes, looks like this is working. Thanks!