Crafty icon indicating copy to clipboard operation
Crafty copied to clipboard

Improve branch management

Open starwed opened this issue 9 years ago • 3 comments

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

starwed avatar Apr 17 '16 21:04 starwed

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 on update branch himself.
  • I wasn't able to push changes from my local repo to the develop branch.
    • 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 develop without going through the process of PRs. Even after Travis checks pass for my branch on my forked origin repo, GitHub won't let me push to Crafty's upstream repo.

mucaho avatar May 03 '16 20:05 mucaho

@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.)

starwed avatar May 07 '16 17:05 starwed

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.

starwed avatar Sep 30 '16 19:09 starwed