I\'ve never found the ideal way to perform code reviews and yet often my custome

Question

I've never found the ideal way to perform code reviews and yet often my customers require them. Each customer seems to do them in a different way and I've never felt satisfied in any of them.

What has been the most effective way for you to perform code reviews?

For example:

Is one person regarded as the gatekeeper for quality and reviews the code, or do the team own the standard?
Do you do review code as a team exercise using a projector?
Is it done in person, via email or using a tool?
Do you eschew reviews and use things like pair programming and collective code ownership to ensure code quality?

Explanation / Answer

At my work we have a very simple rule: changes must be reviewed by at least one other developer before a merge or a commit to the trunk. In our case this means the other person physically sits with you at your computer and goes through the change list. This is not a perfect system, but it has noticeably improved code quality nonetheless.

If you know that your code is going to be reviewed that forces you to look it over first. Many problems become apparent then. Under our system, you have to explain what you did to the reviewer, which again causes you to notice problems you may have missed before. Also, if something in your code is not immediately clear to the reviewer, that is a good indication that a comment is required. And, of course, the reviewer may find problems too. Furthermore, in addition to looking at the change, the reviewer may also notice problems in the nearby code.

There are two main drawbacks to this system. When the change is trivial, it makes little sense to have it reviewed. However, we absolutely have to stick to the rules, to avoid the slippery slope of declaring changes to be "trivial" when they are not. On the other hand, this is not a very good way to review significant changes to the system or addition of large new components.

We have tried more formal reviews before, when one developer would email code to be reviewed to the rest of the team, and then the whole team would get together and discuss it. This took a lot of everyone's time, and as a result these reviews were few and far between, and only a small percentage of the code base got reviewed. The "one other person reviews changes before commit" has worked much better for us.

Related Questions

Navigate

• Browse (All)
• Browse I
• Subjects

Integrity-first tutoring: explanations and feedback only — we do not complete graded work. Learn more.