: Re: Methods for writing a code review I currently spend a good deal of time over at Code Review and I would love to improve the quality of the reviews I write. Can you give me any insight into

@Hamaas631

Vote Up Vote Down

How you structure a code review depends on the tools you're using and the level of scrutiny that was requested. Instead of giving you an exact template, therefore, I'll address the different types of content.

At the lowest level, a code review can include feedback on individual lines or sections of code. Most important is to point out any errors you find; you might also point out undesired consequences (side effects, performance problems, etc). Think of this as the fine-toothed-comb level.

At the next level up, you might comment on some common themes -- concurrency, asynchronicity, pounding the snot out of the server, whatever. These are the kinds of issues you want to talk about broadly instead of pointing out every single place in the code where it happens, so you need to both explain the problem and give the review-ee enough information to be able to find and fix the individual occurrences.

At the highest level, you might have feedback not about the code itself but the design or architectural approach. Ideally there won't be too many of these because you'll have had design reviews earlier on, but sometimes stuff comes up. In deciding how to provide feedback here, ask yourself: how likely is it that anybody can actually act on what I'm going to say? Sure, this would have been better if you'd used a completely different framework or made these fundamental design decisions differently, but if that's not going to change now, there's no point in spending time on it. The purpose of a code review is to improve that code, not wish that completely different code had been written. That said, if there is an opportunity to advance (rather than ditching things and starting over), you probably do want to point that out -- e.g. the upcoming version of this library you're using is going to have feature X, which we could use to simplify this part of your code.

Now, how to present it? The lowest level is best done with some sort of code markup, like that provided by code-review tools. (I'm thinking of, for example, the git diff on pull requests here.) The other feedback, the stuff not tied so deeply into the code, is in my experience best done in a single higher-level document -- which could just be a text file -- that accompanies the code-level comments. This higher-level document might be anywhere from a few paragraphs to several pages, depending on the amount and importance of the code, the magnitude of the issues, and the level of detail requested. Try to establish some shared expectations about this beforehand; getting a tome when you expected a handful of comments or the reverse is going to be frustrating for the recipient.

(0)

Login to post a comment!