How do you do code reviews?

Question

I've recently moved from a company with 30 or so developers all based on the same site, to a smaller company with a total of 6 developers working all over the country.

Previously when I was conducting code reviews I'd have the person sitting next to me, or I'd go round to his/her Pc and go over the code with them.

At my new company this isn't feasible as people are all over the place. I've been handed a couple of thousand lines of code to review and I can't just tell someone my thoughts.

The only way I can think of is to write a code review document. So I've resorted to pasting the code into Word and annotating it using the little margin bubble things you get.

is there an easier way to do this? I've long thought that Visual studio should have a meta comment system, which would give you similar outlining features, and allow you to write comments on a higher level.

Are there any tools that do this? If not how do people go about doing distributed code reviews?

Answer 1

JIRA has the option to assign someone to code-review your work, he/she can then give comments within JIRA.

Answer 2

We were exactly in the same boat - developers all over the country, not enough overlap time during the day... Everyone working asynchronously... So we had to come up with an offline code-review tool.

We ended up with a reasonably light process: - For any non-trivial piece of code, a peer review is mandatory. - To perform a review, the author creates a "packaged changelist" (check your source control community for such stuff). It's basically a single-file, expandable ZIP of all the changes made in this chunk of work. - The author sends an email to the reviewer with the packed changelist (note they didn't submit it yet!) - The reviewer goes through the code, asynchronously, and puts in the comments in an email - If reviewer has serious objections, code is not allowed to go in until another round of reviews; otherwise, author is allowed to check in after they fix the suggestions OR open bugs to fix suggestions.

Answer 3

You might like to read the Smart Bear book: Best Kept Secrets of Peer Code Review

It's free (including shipping if you're in the US) so there's little reason not to get it. On the other hand, it's a great little book with real world examples on how best to do peer code reviews, so there's plenty of reason to order it.

Answer 4

Check out the suggestions from this question:

http://stackoverflow.com/questions/403/tool-to-aid-code-review

Answer 5

http://www.review-board.org/

I saw this project not too long ago, perhaps it can get voted up if people use it.

Answer 6

If possible, start with test cases -- have the person walk you through the test cases. It shows the design and what the developer was thinking would solve the problem(s). Dive into test cases and look at code.

Also, take a look at code coverage reports generated automatically from a build. It can be a good metric on test paths.

That's where I would start.

Answer 7

We have developers based around the world. We use Smart Bear's Code Collaborator. Like any tool, it's not perfect, but it certainly gets the job done. It's web based and easy to learn.

Answer 8

We have had some success using the the TFS shelveset feature to do distributed code reviews. Using shelvesets a developer can 'check in' a changeset without commiting them to the branch. Another developer can then unshelve the changeset and review the diffs against the commited branch. Once everyone concerned is happy with the changeset they are commited to the branch.

Answer 9

There's a tool: http://www.smartbear.com/codecollab.php

Answer 10

A very lightweight process that we have been using is to use Microsoft SharedView and Skype both to have pair programming sessions and to do code review. Granted, it's not as formal as a check-in policy, but might be a good place to start in order to develop standards before memorializing them in a check in policy.

Answer 11

In the company I am working for, patches, generated through SVN, are submitted on a daily basis in a newsgroup. Live code review is still the most efficient way to complete a review however, since we are working on virtual teams with developers all over the world it is sometime impossible to have the developer and the reviewer talk to each other.

To help our developers doing the reviews, we have put together a code review checklist (available at: http://www.macadamian.com/index.php?option=com_content&task=view&id=27&Itemid=31).

Our secret behind code reviews: Review small, review often.

Answer 12

Well, I'll tell you what my company does since it works well for us (about 10 developers plus PM ppl). YMMV.

We do our change management using Subversion and ClearQuest (used to use ClearCase, looking to ditch CQ). We do our changes, create patch files using subversion and then put the patch on the CQ record and pass the CQ around. Low-tech but it works. Obviously you can modify it to work with whatever ticketing app you use.