One of the branching philosophies (section Developing Branching Strategy and Codeline Policy in Advanced SCM Branching Strategies - also read Perforce Best practices, its a pdf but goes into some other details) is that you branch on incompatabile policy.
A codeline policy
specifies the fair use
and permissible check-ins for the codeline,
and is the essential user’s manual for
codeline SCM. For example, the policy of
a development codeline should state that
it isn’t for release; likewise, the policy of a
release codeline should limit changes to
approved bug fixes. The
policy can also describe
how to document changes
being checked in, what
review is needed, what
testing is required, and
the expectations of
codeline stability after
check-ins. A policy is a
critical component for a
documented, enforceable
software development
process, and a codeline
without a policy, from an
SCM point of view, is out of control.
(from Perforce Best Practices)
Say you have the branches 'release' (or 'master') from which a release is built and 'trunk' (or 'dev') where developers check in working code. These are the policies of the branches. Noting the 'working code' is part of the 'dev' branch policy, one should never commit broken code to the dev branch. Often there are things such as CI servers hooked up to these branches and checking in broken code into dev could mess up everyone's branch and break the build.
However, there are times when it is appropriate to check in partial code that doesn't work. In these instances, one should branch - an incompatible policy with trunk. In this new branch, one can decide the policy ('broken code is ok') and then commit code to it.
There is one simple rule to determine if a
codeline should be branched: it should be
branched when its users need different
check-in policies. For example, a product
release group may need a check-in policy
that enforces rigorous testing, whereas a
development team may need a policy that
allows frequent check-ins of partially tested
changes. This policy divergence calls for a
codeline branch. When one development
group doesn’t wish to see another
(from Perforce Best Practices)
Realize that this is coming from a central server based SCM with a strong corporate mindset. The core idea is still good. These are often thought of implicitly - you don't check in untested dev code into the release branch. Thats a policy.
So branch, say that this branch can have broken code and commit away.
The solution I decided was simplest and kept the most advantages for me was to rebase and then merge. After using github to go back and forth on the pull request and committing a few minor changes it was time to merge it into the master branch. On my local setup I did:
git pull
git checkout feature
git rebase -i HEAD~8 # combine small commits and separate file moving
git checkout master
git merge --no-ff feature # comment this with `close #<pull_request_number>`
git push
The advantages are:
- On the master branch the history is cleaner and more concise
- The explicit merge at the end links the pull request to the merging commit
The disadvantages are:
- The rebased commits in the pull request no longer link to commits in the master which could be confusing.
- I didn't actually go back and make sure each of rebased commits works in the new rebased context so small chance of bugs there
Not sure if this is a recommended approach or a little too hacky and I should stick to one or the other. But we'll see if any problems stem from this. Very flexible to different suggestions from other people though.
Best Answer
This workflow gives good results for most large software projects that follow some version of the the Atlassian Git Flow model
Each merge to the branch from which the release is cut must leave the project in a working state. In Git Flow, this is called the
master
branch, and usually only the release engineers can merge code there. Run all your tests for each merge;Each merge to the mainline development branch (in Git Flow it's called
develop
branch, but many use themaster
branch for that purpose) should leave the project in a working state (and it must build at least). Run most of your tests, except the longest running ones, accept some flakiness, don't run unaffected tests. But sometimes you'll merge two branches that both pass tests, but their merge is broken. This is okay. You'll fix this soon, and you'll make sure your release branches are good.Each other individual commit has a primary goal of explaining why the change is made, and what is it for, and what parts of the project it affected. All other goals, such as leaving the project in a working state, are optional.