Scheduled Code Reviews?

Question

Hi,

On our team, we like doing code reviews but because a code review can happen at any time it is jarring when you're in the programming "zone" and get tapped to preform a review. You do the review, which is great, but then it takes you ten minutes before you get in the "zone" again.

I was debating about how best to handle the needs of doing code reviews, but minimizing the effects on production. So I was entertaining the idea of having some kind of schedule: a time where programmers pair up and talk about the day's code, but it does mean you have to wait until review time to do it. So I'm still unsure about it.

So how do you best manage code reviews while reducing interruptions?

Thanks!

Answer 1

You might do it as a check-list kind of thing and let the developers choose their own best time. But there must be a real deadline with real consequences otherwise it won't get done. You could also make a day as "review day", like a "review Friday" or something where you encourage reviews to be performed on that particular day.

Answer 2

I'd recommend doing them when the Programmer has finished adding a unit of functionality, as long as the time estimation for that functionality is less than a day, this works.

Or better yet, do the pair programming thing (giving you practically continuous code review), but still have formal code reviews, just less frequently.

Answer 3

I think the idea is to involve people early and often. Code reviews should be viewed as a chance for collaboration that will ultimately improve the design and quality of the code. If code reviews are just an item to check off of a list, or viewed as an interruption to progress, then I think the spirit of the review is missed.

The best code reviews happen when the person writing the code actually seeks them out. That being said, we tend to have a rule that no code is checked in until it has been reviewed. In that way it is a sort of a scheduled review. However, if you wait until the end, you don't gain the benefit of a code review helping to influence the design.

You could also save yourself the trouble and do some pair programming. You get productivity and review all at the same time ;)

Answer 4

G'day,

I reckon this sort of an impromtu review should only be used when you are having trouble understanding some code.

To get the most from code reviews, they must be scheduled in advance to allowing people to independently have a look through the piece of code under review.

It's amazing how easily a reviewer can be talked into missing errors when the author is right along side them. Given a little bit of time on their own, most reviewers are more likely to pick up errors within the code.

So in a nutshell, schedule the reviews in advance and make sure that the reviewer(s) have enough time to go over the code in their own time.

HTH

cheers,

Answer 5

The trade-off between giving people time to prepare for review and minimizing disruptions is a tough one. The other time trade-off, getting the review done while the code is still "fresh", complicates that more.

I've seen a bunch of groups start down the road with the best of intentions, only to have the practice wither and die because, on net, it's too disruptive.

The best thing I've seen for getting fresh reviews with minimal disruptions to flow is to turn the time crank all the way to the left, and review the code as it's being written, by pair programming, and by writing tests as another form of review. (With tests, reviewers have to worry less about "does this work at all?") An extra set of eyes on the code as it's being written is a great way to catch stuff that would otherwise take more people's time in review, and it spreads knowledge around a lot quicker.

You may still choose to do group reviews of critical code, but with fewer reviews going on, the chance of have people prepared for a special one goes way up.

This is a practice that's demonstrably not for everyone, but the teams I've worked with that have done TDD and pair programming tend to have much cleaner code with lower defect counts.

Answer 6

Disclosure: I work for Atlassian, who sell Crucible, a web-based code review application.

I agree that pair programming is a valuable practice, and that it provides some of the benefits of code review (and of course other benefits unrelated to code reviewing!)

You can reduce the disruption caused to reviewers by using a code review tool, so that each reviewer conducts the review at a time convenient to them. This also removes the problem of the author talking the reviewer into accepting the code.

Answer 7

One way you could look at doing it is "reviewing the checkins". In other words, every day, someone reviews all the checkins and then, if there is something flaggable, goes immediately and talks and/or emails the developer concerned.

There is immediate feedback, the quality is improved because the code is changed and it is fresh in the mind of the developer so he learns from it.

I've found it works very well. One thing which this approach does not solve however, is when a developer checks in a whole body of code. It is difficult, with this approach, to review multiple whole files at a time. That requires another approach, an efficient version of which, I'm still looking for.