Posted by
Jason Cohen
on
Thursday, January 15, 2009
It's not obvious that code review and version control branching policy are related, but I've found that lots of our customers have voluntarily changed their branching habits to reduce developer down-time.
The two processes appear unrelated. Branching has to do with isolating code. Isolation can protect "stable" lines from "active development" lines, can separate teams who want to deliver code at specific checkpoints, and can allow an individual to use version control to manage changes that aren't "ready for prime-time."
Code review, on the other hand, is completely separate. You review code, sometimes before check-in sometimes after, but at any rate you should be able to just layer it on top of your current branching scheme.
And you can! Often there's nothing more to it, especially if you want to review code before check-in.
But wait. Requiring a review before code is committed to version control has a price: What does the author do while waiting for the review to complete? The author can't check code in during this time, so what happens? She can't start working on the next thing, because what if defects were found? She would have to revert those additional changes to make just the fixes in question, and that's a big pain. So she's stuck -- frozen until the review is complete. Bad!
In practice, if all team members are in a similar timezone this isn't much of a problem. We do this at Smart Bear and all you do is use the extra time to catch up on email (since you had email turned off while you were coding in the zone, right?) and take a minute to plan how you're going to do your next task. Reviews usually take fewer than 30 minutes to complete -- faster if you pinged the reviewer to hurry it up -- so no harm done.
But this is not the case with distributed teams. If a developer in Hyderabad finishes a bug-fix at 10am local time, it might not be until the following day before it's reviewed in New York City! Surely the developer can't be idle that long.
This is where branching policy can save the day. Instead of requiring "review before check-in," require that changes be made in a "local sandbox," meaning a little branch made for one developer for the purpose of working on a single task. When the task is done, you review the branch (or the integration of the branch with the mainline, depending on your version control system), rather than reviewing the "changes not yet checked in."
Once the review is set up the developer can start working on something else in a different branch. If the new task is unrelated, this poses no problem. If the new task depends on the changes from the first task, the new branch can be based on the first branch! That way, if fixes come back they can be applied directly (and re-synched with the new task), but if no problems are found the new task proceeds unhindered.
Local sandboxes have other desirable features besides un-blocking early-stage code review. The developer can now use version control for intermediate steps rather than having to wait for days before a large check-in can occur. (This is a problem solved by certain version control systems like AccuRev and with distributed version control systems like git and hg, but really that's because they are often used as a complex local sandbox!)
Also you can have more than one developer using a local sandbox. Perhaps two or three developers are working together on a new feature, or perhaps a Scrum group shares a sandbox until they are nearing the end of the iteration and want to do code reviews with each other as part of the definition of "done."
The downside to all this is the cost of having more branches on the version control server. Some systems are better than others at scaling to a large number of branches, particularly when they contain large numbers of files. To some extent this can be managed by pruning old branches and detecting when a branch has been started but abandoned, but this is all work that someone has to do.
Still, in the end the purpose of your IT infrastructure is to support software development. Developers must not be unreasonably blocked from writing code. The cost of upgrading some servers is worth it. Besides, increasing demand on IT systems means job security for IT guys, no matter how they might protest. :-)