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

Protected branches #267

Closed
dcousens opened this issue Sep 21, 2015 · 11 comments

Comments

@dcousens
Copy link
Member

commented Sep 21, 2015

@feross any chance we could protect master such that PRs are still the norm?
I'd rather not keep up with the commit log directly if possible.

@dcousens dcousens added the question label Sep 21, 2015

@rstacruz

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Protected branches won't prevent people from directly committing to master, only push --forces.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Sep 21, 2015

@rstacruz true, for some reason I mis-interpreted:

Choose which status checks must pass before branches can be merged into master. When enabled, commits must first be pushed to another branch, then merged or pushed directly to master after status checks have passed.

To mean that you can only merge, with no direct commits, to master.
Well that sucks.

edit: as pointed out below, it does enforce the behaviour I want, provided you have required status checks.

@dcousens dcousens closed this Sep 21, 2015

@Flet

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Its probably still a good idea turn on protection to disallow push --force :)

@feross

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Enabled it:

screen shot 2015-09-21 at 10 03 56 am

@Flet

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Ah, so the status checks option is close to what @dcousens was asking for? If I'm reading it right, then it means everything must be pushed to a separate branch first to allow checks to run...

@feross

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Yep, seems that this does what @dcousens wants.

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Required status check "continuous-integration/travis-ci" is expected
To git@github.com:feross/standard.git
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'git@github.com:feross/standard.git'

Happy to try it out for a while. I like the idea that it increases visibility of changes by making it impossible to sneak in commits without people getting email notifications.

@rstacruz

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

👍

@Flet

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Yeah, lets try it!

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Sep 22, 2015

@rstacruz right, so it seems that specific option allows you to push to master unless you have status checks.
That aligns with my initial impression, it was just that I've thus far only used this feature with status checks. My quick trial to test the above was without status checks, and therefore confused me.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Sep 22, 2015

@feross for it to align with your idea that those with access are still free to make changes, it just means they can self-merge.
Its still a win for visibility.

@feross

This comment has been minimized.

Copy link
Member

commented Sep 22, 2015

Yep, definite win for visibility.
On Mon, Sep 21, 2015 at 6:32 PM Daniel Cousens notifications@github.com
wrote:

@feross https://github.com/feross for it to align with your idea that
those with access are still free to make changes, it just means they can
self-merge.
Its still a win for visibility.


Reply to this email directly or view it on GitHub
#267 (comment).

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
4 participants
You can’t perform that action at this time.