search

awslabs / aws-shell

An integrated shell for working with the AWS CLI.
Apache License 2.0
7.15k stars 770 forks source link

Enable protected branch for master? #64

Closed donnemartin closed 8 years ago

donnemartin commented 8 years ago

Protected branches are a relatively new and handy feature:

https://github.com/blog/2051-protected-branches-and-required-status-checks

At the very least I think we should enable these for master:

This is also handy too:

jamesls commented 8 years ago

:+1: Haven't actually tried this on any repos yet, but seems like it's worth doing.

jamesls commented 8 years ago

Enabled. Also enabled the required status checks pass. Let's see how it goes.

jamesls commented 8 years ago

Hmm, just ran into the first snag. Trying to merge this: https://github.com/awslabs/aws-shell/pull/73, github prevents me from merging because there's been other commits to master since the PR was sent.

I get why it's rejecting the push. It's possible that the subsequent commits from master when integrated with the PR branch could cause test failures.

So if I'm understanding correctly, every pull request will become unmergeable as soon as we push any new commits to master. =/ So the options:

  1. Require users to click the update branch, which merges master into the PR branch. Not my preference, it clutters history. I'd prefer they rebase against master.
  2. Require users constantly rebase as we push commits to master.
  3. Pull their branch down locally, rebase against master. Push up to github as our own branch. Wait for travis to pass. Then merge to master.
  4. Not have the required status checks.

What I'd like is something like https://github.com/barosl/homu, which can run the tests and merge for you.

I'm thinking I'm going to disable the required status checks for now.

donnemartin commented 8 years ago

Seems that feature could use an intermediate level to be more practical.

https://github.com/barosl/homu

Awesome...starred.

I'm thinking I'm going to disable the required status checks for now.

I agree I think this is currently the best option.