Talk About Quality

Tom Harris

Continuous Code Review

with 3 comments

Continuous code review is reviewing all the code, all the time.

That’s the theory. In real life, some questions come up:

How do we track which code has been reviewed?

All the code?

And when is all the time?

Question 1: How do we track which code has been reviewed?

Answer 1: One cannot, and need not, mark code as reviewed.

One cannot, because, for example, if one reviews a file when it has 100 lines of code, and then later it has 120 lines of code, one may have to re-read all 120 (probably more quickly since much is not new) in order to comment.

And code is not reviewed line by line, but idea by idea.

Use a code editor that maintains a symbol tree so that the reviewer can jump from use to definition and back.

One need not mark code as reviewed because all code should be reviewed.

Question 2: All the code?

Answer 2: Yes, all the code that has been included in continuous code review.

During start-up, one might mark components or modules as having been included in the continuous code review process.

In steady state, by definition, all code is reviewed.

Question 3: When is all the time? Pair programming? Or surely, before every check-in?

Answer 3: None of the above. Try once a week.

The idea is to direct a constant flow of intelligent, objective comments to the developer, as soon after coding as possible.

Code review is not testing, and it is not certification. It is a tool to help the developer fix (improve) the code s/he has just written, to improve the code s/he is about to write, and in general, for the developer to become better at detailed design and coding.

Pair programming is not code review because the “non-driver” is not objective—s/he is part of the pair.

And it does not matter whether code is ready for check-in or not.

All you need is enough newly-written code for a reviewer to understand what the developer is trying to accomplish.

Get started!

Advertisement

Written by Tom Harris

January 30, 2007 at 12:33 am

Posted in Code Review

3 Responses

Subscribe to comments with RSS.

  1. Hi Tom,

    You are right on every account. I would even say (assuming nobody is hearing) that some parts of the code (even large parts) are reviewed more than once — each time in a different context, or as part of a different interaction (with other parts). So in fact, you review 150% of the code. But I would settle for 100% as a start 😉

    Lidor

    Lidor

    February 12, 2007 at 2:18 pm

  2. I like this post, but I have to disagree about reviewing once per week instead of before check-in.

    The problem with delaying the review is that problems that could have been discovered and fixed are compounded. New code builds on a bad decision and now the fix takes much longer to implement.

    I would agree that if you do a reasonable amount of technical design before starting, then your way is best.

    Jason Cohen

    December 15, 2008 at 8:40 pm

  3. The advantage of review before check-in is that you keep the codebase clean. That’s what you’re saying. The disadvantages of review before check-in are that reviewer availability can delay check-in, and that such reviews are shorter and more superficial, concentrating only on the change to be checked in.

    Really, both pre-check-in and weekly are needed, and I’ve seen an organization that tries to do that. Last I checked, they were succeeding on pre-check-in reviewer availability (because the company culture supports it), and perhaps the weekly too, thanks to a manager who monitors open in-depth review requests.

    If I have to choose … well, it’s hard to choose. Both are needed.

    As for up-front design, of course you’re right, but it can be hard to enforce in practice. The weekly code reviews, then, are in-depth enough to find the design problems and justify why up-front design and design review should be done the next time …

    Tom Harris

    December 16, 2008 at 6:57 pm


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

%d bloggers like this: