Improve branch management
As an experiment, I set the develop branch to protected, and added requirements for merging braches into it. They're the same ones we should already be using in principle. :) If it works as I expect, Github will now require that a branch:
- is up to date with develop
- has passed the Travis CI checks
It'll also prevent accidental force-pushing into the develop branch. I didn't protect the master or testing branches, because we could conceivably want to force push into those to fix mistakes.
I opened this issue to track any suggestions or problems this causes!
@craftyjs/contributors
This is good, except for these inconveniences:
- If I want to merge in someone's changes into develop, I can't because they are not up to date with develop. I am unable to update that person's branch to newest develop revision from GitHub's interface.
So, in order to merge in those changes, the author of those changes needs to click onupdate branchhimself. - I wasn't able to push changes from my local repo to the
developbranch.- So, if a contributor is inactive, I can't even update his branch locally and push it manually :(
I can, however, create a new PR that contains the updated commits. - Additionally, sometimes I like to directly push some small changes from my local repo to
developwithout going through the process of PRs. Even after Travis checks pass for my branch on my forkedoriginrepo, GitHub won't let me push to Crafty'supstreamrepo.
- So, if a contributor is inactive, I can't even update his branch locally and push it manually :(
@mucaho Those seem like valid points. It's probably not worth mandating the up-to-date checks if that's causing problems with accepting PRs; we haven't had a lot of issues with conflicts happening in Crafty.
I left develop protected, but turned off the requirement for status checks to pass -- I believe that this will let you make regular pushes to develop, and will only prevent force pushes.
Hopefully github will improve the ergonomics of the status check feature in the future, and we can make use of it. (I think it was added extremely recently.)
There's a new "Require pull request reviews before merging" option available on protected branches, but I think that in practice that might just be annoying.
For now we'll just leave it as is, but as always, I'm open to suggestions here.