I’m working on implementing code reviews for a client, and dug up this old mind map of what I consider important.
Let me know if you have any suggestions on improving it.
Click to expand
- How to do a code review
- Select the code to review
- Ensure all developers eventually have some of their code reviewed
- Indicators of good code to review
- Code that already has problems
- Bugs
- Performance issues
- Crucial code
- Complicated code
- High cyclomatic complexity
- Long classes and methods
- Large amount of class coupling
- Code that could be re-used in a library
- Code that already has problems
- Review limits
- Review approximately 300 lines of code
- Select one class, or a few related classes
- Group review meeting is one hour long (or less, if done before one hour)
- Who should attend reviews?
- Author of code being reviewed
- Moderator
- Not the code author
- Ensures review stays on track
- Writes down changes to make
- A mix of junior and senior developers (to help spread knowledge)
- Author goes through the code before sending to reviewers
- Add a short description of what the code does
- Also gives them a chance to personally review the code once more
- Send code to reviewers before the group review
- Two or three days before the group review
- Do individual, quick pre-reviews before the group review
- Make notes for the group review
- During the group review
- Go through [roughly] 10 lines, or one method, at a time
- If printing source code, include line numbers
- No personal attacks on the author are allowed
- Don’t make changes based on other programmer’s personal preferences
- Moderator records changes the team agrees on
- To the source code
- To the coding standards (if applicable)
- To the continuous integration process (automated checks)
- Go through [roughly] 10 lines, or one method, at a time
- After the group review
- Ensure changes are made and checked in
- Select the code to review
- Benefits you’ll receive
- Defects are found before going to QA (or production)
- Generally, more defects are found here, than in QA
- Code reviews are more likely to find “edge case” problems
- More developers are familiar with the code
- Developers share “best practices” with each other during the review
- Potential problems
- Some developers use reviews to show their “superiority” over other developers
- Reviewers lose focus and start discussing larger, or unrelated, issues
- Reviewers state personal preferences as “the best way”
- Suggested changes are never made
- Some reviewers don’t contribute any suggestions
- Potential problem solutions
- Have phrases to stop discussion problems
- “Offline”
- “Personal Preference”
- Have phrases to stop discussion problems
- Modify the process
- If you find recurring problems, can you add automated checks for them?
- Would you do better with fewer, or more, reviewers in each group review?
- Do the individual pre-reviews lead to better group reviews?
- What NOT to do
- Use reviews to evaluate developers
- Decreases desire to participate
- Less participation = fewer bugs found
- Use reviews to evaluate developers
- Types of defects
- Bugs/errors
- Poorly named classes/properties/methods/variables
- Duplicated code
- Code that will cause performance problems
- Code that doesn’t match coding standards