Branch protection settings

[RalfJung]

After a lot of searching I found the recommended branch protection settings for bors, somewhat hidden in a section titled "If it doesn’t work":

You can check this on GitHub in your repository’s Settings tab, in the Branches section. The “master” branch can be protected, and since bors will usually be the only thing that commits directly to master, you can set it to require the “bors” Commit Status to push to master. Do not set the staging/trying branches to protected.

However, this "require commit status" has a very annoying side-effect: when opening a PR, even after local CI in that branch passes, GitHub considers the branch as "waiting for CI" and will not show a green checkmark. So, I think setting that option is not good advice.

Further down in that section, after some unrelated answers, it also says

Also, make sure bors is included in the list allowed to push to the protected branch.

I assume that refers to "Restrict who can push to matching branches"? Having an explicit statement in a single place, maybe even a screenshot, of what the recommended branch protection settings are would be really helpful. :)

[jethrogb]

See #576

[RalfJung]

The "bug in GH" link in that issue is dead, so unfortunately this does not explain anything.

What's wrong with enabling "Restrict who can push" and then disabling "Require status checks"?

[jethrogb]

Here's the Internet Archive version: https://web.archive.org/web/20190214183904/https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/1376/56

"whatever rust-lang/rust does" should be good enough for us, I think.

Rust-lang/rust uses a GH user named bors. If you use this (bors-ng), there's only a GH app integration. GH users are have security properties compared to GH app integrations.

I am not sure what they do, but I'd enable "Restrict who can push to matching branches", set that to bors-only, and also enable "Include administrators" to enforce the restrictions for admins. (Well personally I think admins having an override is fine, but they do anyway -- they can change the branch protection settings.) I see no good reason to enable "Require status checks to pass before merging".

IME, If you set it up that way, administrators are not actually prevented from pushing to master. GitHub is just broken this way.

Try setting up bors+branch protections on a repo you own and see if you can get it to work. The setup must not allow any of the following things:

• Pushing a new commit to master
• Clicking the merge PR button in the GH UI before Travis passes
• Pushing a commit that has an open PR to master before Travis passes on that PR
• Clicking the merge PR button in the GH UI after Travis passes
• Pushing a commit that has an open PR to master after Travis passes on that PR

The following must be possible:

• bors must be able to push to master when it wants to

[RalfJung]

Actually I am beginning to seriously wonder what rust is doing. Because it looks like "Restrict who can push to matching branches" automatically includes repo admins no matter what.

@pietroalbini any chance you could share which settings the rust-lang/rust repo is using for branch protection to prevent anyone but bors from lading stuff?

[pietroalbini]

We don't do anything special in rust-lang/rust, admins of that repository see the green merge button. We just avoid clicking it, and we didn't have many misclicks in the past.

[RalfJung]

Oh I see. Well looks like there is no perfect solution then... :/

Maybe the issue could remain open to at least more clearly document the recommended branch protection settings? As I mentioned, I found it quite hard to find them.