-
Notifications
You must be signed in to change notification settings - Fork 64
Branch Policies
A lot of this will be stuff we're already doing, but I thought I would quickly lay it all out so that we have it in one place. This also allows us to discuss some of the weaknesses of this approach which have emerged and how to best address them.
General policy: if you push a branch to GitHub, do not rebase or amend the commits. The exception to this is if you push your branch with the "wip/" prefix. We have, at present, three conventional prefixes:
- topic/ – For feature branches
- bug/ – For bug branches
- wip/ – For work in progress that was pushed merely as a sharing mechanism
It should always be safe to base work off of a topic/ or bug/ branch. Thus, if there's a topic/ branch that has commits you need (perhaps because it has yet to be merged), go ahead and cut your new branch from that old one.
We should not be basing off of wip/ branches. wip/ branches are for sharing code between a few specific people. It's using git push as a transport mechanism, rather than a publishing mechanism. Thus, wip/ branches are not safe for branch basing, but they are safe for rebasing and amending (for the same reason). If you have some commits that you feel the need to push, but also feel like you might need to rebase in future, push them as a wip/ branch.
Overall, I think this has worked pretty well. The biggest problem I've seen with this strategy is it is very difficult to fix mistakes in a push without adding commits. Specific scenarios:
- Minor changes from code review (e.g. typos, code style, etc)
- Missing Pivotal annotation on commit message
- Spurious checkpoint commits that weren't squashed
All of these are things that would normally result in a rebase/amend, and honestly I've done it once or twice on topic/ branches that I've pushed and caught very quickly. However, we really shouldn't be doing this at all. The merge hell that Derek hit today was caused by me when I asked Brian to amend one of my earlier commits with a [Finishes …] annotation that I had forgotten. I thought it was safe, but Kris had already based off of that branch, hence the situation we're in now…
The sorts of scenarios I enumerated do and have happened, so we need some way to sanely deal with them. Here's my proposal: If we catch a mistake quickly (as in, within the first minute after pushing), we should immediately delete the remote branch we just pushed and perform the appropriate rebase/amend locally, subsequently re-pushing. If it's something that isn't caught until more than a minute after pushing, assume that someone else has already based on that branch and leave it alone. If you're trying to make minor code-review fixes (as in the first scenario), just put it in a new commit and push that (rather than amending). If you forgot to add a Pivotal annotation, you will need to remember to go in and click "Finish" on the appropriate task. In the case of checkpoint commits, we'll just have to deal with that bit of ugly history.
When in doubt, just don't rebase/amend. I underestimated how much people were basing off topic/ branches, which is how we got into the predicament with pull request #113. The assumption really should be (with topic/ and bug/ branches) that any work you push becomes the foundation for something someone else is doing almost immediately.