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

Locking down the master branch #12771

Closed
larsbergstrom opened this issue Aug 8, 2016 · 14 comments
Closed

Locking down the master branch #12771

larsbergstrom opened this issue Aug 8, 2016 · 14 comments

Comments

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Aug 8, 2016

We already have disabled force-push to master and prevent it from being deleted in servo/servo.

I'd like to propose:

  1. No "require status checks before merging", since the CI handles that. Same as today.

  2. DO add "restrict who can push" and make it exclusively bors-servo. Different from today.

I am assuming that by adding the restriction to push, it will also disable the Big Green Button for everybody. I'm attaching a screenshot of the options we have available:

branchprotect

cc @metajack @edunham

Might also be interesting to @alexcrichton @brson

@highfive
Copy link

@highfive highfive commented Aug 8, 2016

@larsbergstrom
Copy link
Contributor Author

@larsbergstrom larsbergstrom commented Aug 8, 2016

Oh, one icky bit is that you can't remove the "organization and repository administrators." So it'd be @bors-servo + servo/Administrators. Currently, that's only six of us, which is much better, but still not as small as would be ideal.

@metajack
Copy link
Contributor

@metajack metajack commented Aug 8, 2016

That seems to drastically reduce the surface area, even if we can't disable administrators. +1 from me.

@alexcrichton
Copy link
Contributor

@alexcrichton alexcrichton commented Aug 8, 2016

@larsbergstrom thanks for the heads up! This is actually exactly what we have on rust-lang/rust right now, and we're also sad that we can't turn off our own push access :)

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Aug 8, 2016

I would not want @bors-servo to be the only account with push access because occasionally we may need the larsbors.

@larsbergstrom
Copy link
Contributor Author

@larsbergstrom larsbergstrom commented Aug 8, 2016

@KiChjang fortunately, I have the @bors-servo password and can pretend to be a robot with the best of 'em :-)

@larsbergstrom
Copy link
Contributor Author

@larsbergstrom larsbergstrom commented Aug 8, 2016

@edunham pointed out that we should probably do this for all the repos in homu's cfg.toml that we run the autolander on.

@larsbergstrom
Copy link
Contributor Author

@larsbergstrom larsbergstrom commented Aug 9, 2016

OK, after chatting with @edunham I've rolled this out for servo/servo. We can follow up on the other repositories.

@wafflespeanut
Copy link
Member

@wafflespeanut wafflespeanut commented Aug 9, 2016

That big green button was truly itching (and scary!) 😄

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Aug 9, 2016

It looks like there is an experimental GitHub API we could use to enforce some of these permissions automatically (via Homu? via cron?).

@notriddle
Copy link
Contributor

@notriddle notriddle commented Aug 13, 2016

That API doesn't let you do anything you can't do from the UI anyway.

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Aug 13, 2016

@notriddle right, but being able to do it via an API is nice for two reasons:

We use tons of repos, so automating settings like these is a time-saver (and also useful for auditing and consistency).

@edunham
Copy link
Contributor

@edunham edunham commented Jul 14, 2017

branch protection etc tracking issue is over at #17626

@nox
Copy link
Member

@nox nox commented Oct 4, 2017

This has been done after I pushed a rustup on master in a moment of complete confusion. Teehee.

@nox nox closed this Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.