NaN code review procedure

Version: $Id: review.html,v 1.3 2001/03/05 12:50:23 chrisp Exp $
It is good practice to let your code be reviewed by other people before commiting changes.

Index

1. Why do I want to have my code reviewed?
2. What should I do to do a good review?
3. Some advice on reviewing?

1. Why do I want to have my code reviewed?

Before you commit changes to the code, it is good to let someone else inspect your code. This does two things:

someone else gets to admire and learn from your beautiful creations,
that person can find small mistakes (like mixing x-s and y-s, or missing that annoying ;)

Ask any other developer to do the review (it is supposed to be a peer-to-peer review after all).

2. What should I do to do a good review?

There are two situations in which a review is in order: when creating code from scratch, and when adding/modifying existing code (e.g. to fix a bug). Apart from the amount, there is no difference. It is often helpful to do the review in the presence of the person who made the changes, so you can ask him about what he did, and why.

Reviewing is very simple: read the changes, and think about them for a while! Make notes of the mistakes you find, and send/give them to the author. Draft a little report, and send it to [email protected]. The report needs to contain the following information:

your name,
date of the review,
whose code did you review,
how many lines were reviewed,
how many mistakes/defects you found (the coder wants to know this as well),
how much time you spent on the review,
which files were reviewed (When reviewing partial files: function names or line numbers, but don't get hung up on this. I use this info to do statistics on the code, so ...).

I use this data to monitor the software engineering process.

3. Some advice on reviewing?

Take you time when reviewing: you can review about 150 lines per hour effectively. Reviewing faster will make you miss most mistakes.
There are tools that can help comparing the differences in two files (especially useful when looking at bug fixes). I am still looking for a good tool to introduce here (I have a sort-of working tool. Using diff can also help.).
Plan reviews! It takes some time to review, and other people may not have time immediately. Doing a review can be postponed, but this is only useful if no other changes are being made on the code in the mean time. One of the aims is finding mistakes before they start bugging someone, so we want to catch them before the code is used, either by other coders or by users.
Reading code is different from writing it in a number of ways. Please take reviewing serious. A good review team manages to remove more than 95% of all errors in one review pass! It takes time to learn how to do it properly.