Thursday, October 8, 2009

Refactoring work for review (and keep your annotations)

Tim Penhey recently had a nice post about how he split up his changes to make it easier to review. His method used 'bzr pipeline' and some combinations of shelving, merging, and reverting the merges.

However, while I wanted to refactor my changes to make it easier to review, I didn't want to lose my annotation history. So I took a different approach.

To start, with, I'll assume you have a single branch with lots of changes, each wrt different features. You developed them 'concurrently' (jumping back and forth between features, without actually committing it to a different branch). And now that you are done, you want to split them out again.

There are a lot of possible ways that you can do this. With some proponents prefering a 'rebase' style. Where you replay the commits you made in a new order, possibly squashing them, etc. I'm personally not a big fan of that.
Tim's is another method, where you just cherrypick the changes into new branches, and use something like bzr-pipeline to manage the layering. However in reading his workflow, he would also lose the history of the individual changes.

So this is my workflow.
  1. Start with a branch that has a whole lot of changes on it, and is essentially 'done'. We'll call this branch "dogpile".
  2. Create a new branch from it (bzr branch --switch ../dogpile ../feature1), and remove all of the changes but the 'first step'. I personally did that with "bzr revert -r submit: file1 file2 file3" but left "file4" alone.
  3. "bzr commit" in that branch. The delta for that revision will show a lot of your hard-worked on changes being removed. However "bzr diff -r submit:" should show a very nice clean patch that only includes the changes for "feature1".
  4. Go back to the original dogpile branch, and create a new "feature2" branch. (bzr branch --switch ../dogpile ../feature2)
  5. Now merge the "feature1" branch (bzr merge ../feature1). At this point, it looks like everything has been removed except for the bits for feature1. However, just using "bzr revert file2..." we can restore the changes for "feature2".
  6. You can track your progress in a few ways. "bzr diff -r submit:" will show you the combine differences from feature1 and feature2. "bzr diff -r -1:../feature1" will show you just the differences between the current feature2 branch and the feature1 branch. The latter is what you want to be cleaning up, so that it includes all of your feature2 changes, built on top of your feature1 changes. You also have the opportunity to tweak the code a bit, and run the test suite to make sure things are working correctly.
  7. "bzr commit" in that branch. At this point, the diff from upstream to feature1 should be clean, and the diff from feature1 => feature2 should be clean. As an added benefit, doing "bzr annotate file2" will preserve all the hard-won history of the file.
  8. repeat steps 4-7 for all the other features you wanted to split out into their own branches.
When you are done, you will have N feature branches, split up from the original "dogpile" branch. By using the "merge + revert things back into existence" trick, you can preserve all of the annotations for your files. This works because you have 2 sources that the file content could come from. One source is the "dogpile" branch, and the other source is a branch where "dogpile" changes were removed. Since the changes are present in one of the parents, the annotations are brought from there.

This is what the 'qlog' of my refactoring looks like.

The actual content changes (the little grey dots) actually span about 83 commits. However, you can see that I split that up into 6 new branches (some more independent than others), all of which generate a neat difference to their parent, and preserve all of the annotation information from the full history. You can also see that now that I have it split out, I can do simple changes to each branch (notice that purple has an extra commit). This will most likely come into play if people ask for any changes during review.

2 comments:

bignose said...

Thanks very much for this, it's a good description of a workflow I often use in Bazaar but hadn't really thought to contrast with other ways.

The “rebase style” operation seems to be the *only* way to do this in some VCSen, and I've taken your post as a point to discuss with users of those tools.

lrozario said...

Hi,
I am Llewellyn F. Rozario, a Technical Editor with Packt Publishing.

There's work begun to develop a title on ‘Bazaar 2 Version Control’ and I am now looking for an author to

write this book.

Please contact me if you are interested in writing this book.

Thanks,
Llewellyn F. Rozario
[Packt Publishing]
Email: llewellynr@packtpub.com
Web: www.packtpub.com