Code Review: Should the results of a code review count against you?

Code Review: Should the results of a code review count against you?
No.

Quote:
“Code reviews often start off on the wrong foot because they are perceived as an unnecessary step that has been forced upon the developers or, in some cases, evidence that management doesn’t trust the developers. Neither of these perspectives is accurate. Code reviews are a proven, effective way to minimize defects. Whatever additional motivations the organization has for performing code reviews, they are, at their core, an industry best practice.
– Robert Bogue

Application:
Catching defects this early in the software development process saves the company money and results in a much higher quality product. A peer review should never be associated with lack of knowledge or confidence, but with openness and a willingness to learn best practices as a group.

References
http://smartbear.com/resources/whitepapers/best-kept-secrets-of-peer-code-review

Posted in Software Architecture | Tagged , | Leave a comment

Code Review: How do I prep for a code review?

Code Review: How do I prep for a peer code review?
You should create a self-checklist with common mistakes that you have made in the past and use this to help remind you of things to look for in your own code. Try to keep your checklist below 20 items. Additionally, you should run whatever code analysis tools you have at your disposal.

Quote:
“… we believe that requiring preparation will cause anyone to be more careful, rethink their logic, and write better code overall.”
– Jason Cohen

Application:
Your self-checklist is your own private checklist. This is not the same thing as keeping a general “top 10” list of things to look for across the organization.

In regards to code analysis tools, you should run the “Analyze Solution for Code Clones”, run the “Analyze Code Coverage” on your unit tests, clear all code analysis issues detected by Resharper (or run Code Analysis on your code using Visual Studio if Resharper is not installed), and make sure all build warnings have been cleared.

References:
http://smartbear.com/resources/whitepapers/best-kept-secrets-of-peer-code-review

Posted in Software Architecture | Tagged , | Leave a comment

Code Review: What do you review in a peer code review?

Code Review: What do you review in a peer code review?
All changes made should be reviewed in a code review; however, it is normal to focus in on key areas. It is okay for the author to give some hints at something he wants the reviewer to specifically focus on.

Quote:
“We look at everything, but generally focus on the more interesting areas. What’s interesting? Whatever jumps out at us when we’re looking at the code, or anything the author specifically wants to focus on. Our goal isn’t necessarily to put a microscope to every single line of code.”
– Jeff Atwood

Application:
All changes means that all of the following should be reviewed: CSS, HTML, JavaScript, C#, config files, unit tests, and behavior tests. If you need some guidance as to what to look for in a code review, I would advise looking at the Coding Practices covered earlier in the Road to Excellence series.

References:
http://www.codinghorror.com/blog/2006/01/code-reviews-just-do-it.html

Posted in Software Architecture | Tagged , | Leave a comment

Code Review: How long is a peer code review?

Code Review: How long is a peer code review?
The code review process has been researched and studied since 1976, and it has be established and re-established that the maximum product code inspection rate is 200 lines of code per hour.

Quote:
“This is direct and conclusive evidence that reviews should be limited to around one hour, not to exceed two hours.”
– Jason Cohen

Application:
I’ve observed that the average developer creates around 100 lines of code per day in an agile environment. So I expect the average initial review in a peer review to last about 30 minutes. Now since there is usually a cycle of inspection, clarification, and rework, the end to end process of a peer review will vary based on the amount of back and forth clarification communication and the amount of rework that needs to be done. Teams should plan accordingly.

References:
http://smartbear.com/resources/whitepapers/best-kept-secrets-of-peer-code-review

Posted in Software Architecture | Tagged , | Leave a comment

Code Review: When do you do a peer code review?

Code Review: When do you do a peer code review?
A peer code review can be done prior to check-in or after check-in.

Quote:
“I find most people come down strongly on one side or another; and since we make a code review tool, this is a point of contention for a lot of our customers.”
– Jason Cohen

Application:
I advocate doing pre-commit reviews.

By doing the code reviews prior to check-in, you avoid the scenario of a developer repeating or growing an issue that could have been caught during a review process. Although a pre-commit review can create blocking/delay issues, this can also be seen as beneficial as it provides some incentive to get the reviews done quickly and avoid eternally open code reviews.

Post-commit reviews can make it slower and harder to fix bugs, because the reviews will tend to come later when the topic is not as front of mind. Also, post-commit reviews can lead to batches of code that are too big to review.

References:
http://blog.smartbear.com/software-quality/bid/170315/Code-review-before-or-after-check-in

Posted in Software Architecture | Tagged , | Leave a comment

Code Review: Does the reviewee have to be present at a peer code review?

Code Review: Does the reviewee have to be present at a peer code review?
A peer code review can be done shoulder to shoulder or pass around via e-mail/TFS. Over the shoulder reviews do require the reviewee to be present and have the benefit of greater information sharing and learning. The drawback is that they can result in scheduling challenges and disruptions.

Pass around code reviews don’t require the reviewee to be present, and thus don’t have the scheduling challenges that shoulder to shoulder reviews have. Also, pass around reviews provide a record of the review without additional overhead.

Quote:
“Tool assisted reviews strike a balance between time invested and ease of implementation.”
– Jason Cohen

Application:
I would recommend performing 80% of code reviews via a tool assisted pass around style of process like the one provided by TFS 2012. The remaining 20% of code reviews should still be tool assisted, but doing the review side by side. Doing this type of mix should maintain the balance between optimal use of time and maximizing learning opportunities.

References
http://smartbear.com/resources/whitepapers/best-kept-secrets-of-peer-code-review

Posted in Software Architecture | Tagged , | Leave a comment

Code Review: Who should be involved in a peer code review?

Code Review: Who should be involved in a peer code review?
A peer code review involves another developer on the same team (peer), regardless of seniority. A junior developer can learn while reviewing a senior developer’s code and can still have good insights to add.

Quote:
“Code reviews can have more-extensive benefits for a development team. When a junior developer has to review a senior developer, it’s a learning process for both developers. “ The junior developer can learn from reviewing code with a more experienced person,” says D’Souza. “At the same time, when you’re a senior developer and you have to explain your own work, the light bulb might go on that your solution isn’t as solid as you thought.”
– Martin D’Souza

Application:
When selecting a person to perform a code review on your code, you should submit your request to your project team. If you notice that the same person reviews your code each time, request a different reviewer, as you will want to get a mix of feedback over time to grow your skills and to get a fresh perspective.

References:
http://www.oracle.com/technetwork/issue-archive/2013/13-jan/o13upclose-1886634.html

Posted in Software Architecture | Tagged , | Leave a comment