The Code Review you are going to write is going to be for your colleague, a teammate, possibly even your friend. What ever goes into the code review should be done with sensitivity for the work the person is asking for a critique on. Keep in mind they might do your next code review.
You also have an external audience you are doing the code review for. There may be other reviewers. Managers may take a look also, so be professional. Don’t add commentary on policies, rules, or architecture that give the appearance of negativity.
It is very difficult to determine what to put in code reviews. I discussed in a code review post about defining a checklist and some key questions that could be asked. I believe it is necessary to start with a checklist of some type for consistency. Here are other tips around what to put into a code review:
- Summarize the code review that you are doing. If you have a system such as TFS, you could attach it to the work item that you are reviewing. The goal is to provide as much context about the code review.
- Prioritize the list – Put the items that are most critical at the top of the review. Maybe you have a policy that that will log the code review items as tasks to complete. If so attach the whole code review to those items also.
- Identify the line(s) of the error. If possible link to the exact location of the error.
- Provide a potential solution suggestion if applicable. Everyone sees the world differently. It is no different with code. You may have experience that gives you a different context that should be shared.
- Highlight the good things about the review. Share them with the whole team also. This will provide encouragement to the user who did the code.
Below is an example of how an item could look in the code review:
|Reviewed Item: There is an unused parameter. append is never used
Location: XClass at line 45 in Method SaveFile(string x, bool append)
Observations: It is obvious of the intent of Y in terms of append. Great job in thinking about the upcoming features. Suggestion, add the branches to determine whether to append or not and alert the user that the append feature is not implemented yet through an exception.
Code Reviews should be an iterative process in themselves. One of the fatal mistakes that developers have is ignoring code reviews until the last days of the cycle. A large part of this is due to how the features / tasks may be created. Here is a timeline that I would recommend for doing code reviews.
- If a feature is more than “X” amount of time, get a buddy check. X time for me is more than two days on the same feature. A buddy check is a simple 5 minute check to see if structurally or architecturally I am doing something wrong. It also gives me a chance to talk to another person if I am the only one heads down coding.
- Allocate time for the code review in your estimates. Don’t do “dart board” estimating. Ideally code review time should be accounted for in each estimate of work. I have found that a 1/10 ratio on code review time is pretty optimal but that is just my experience.
- Know when to fix the issues from code reviews in current iteration and know when to pass them off to the next iteration. This is a fine line in which you should lean on the side of fixing during the current iteration. Otherwise it becomes a technical debt issue.
Please don’t put code reviews in an email. If you don’t have a documentation system associated with the work you are doing get one. It is everyone’s responsibility to provide the documentation necessary to make informed decisions. At the very least have a central repository where code reviews can be kept. Will anyone read them? I don’t know, but I have worked in one company that highly encouraged new employees to read the code reviews. It had a profound affect on developer coding styles from that point on.
There are so many research studies that have concluded that the best defense against bugs / defects are code reviews. If that isn’t good enough for you here are a couple of other effects of an established code review system:
- Consistency in the code base.
- Shared knowledge across the code.
- Better maintainability when code reviews are done consistently.
- Great opportunities for growth and learning come from doing code reviews, more so for the reviewer then the person that wrote the code.
- You are contributing to the sanity of other developers who might have to modify the code long after you are gone.
Here are some ideas on code reviews in general. Could you benefit from being more rigorous in your code review processes? What ways have you found to be most effective in performing code reviews? What are your reasons for doing a code review?