Code Review Best Practices

Image Credit : https://www.flickr.com/photos/smitty/2245445147/

Code Review Guidelines

1. When should we perform code review?

• Ideally before every single code commit

The following code changes can be excluded from code review sessions

• UI (i.e. HTML/css) only update
• Unit test update
• Urgent fix (prefer pair programming in these cases)
• Auto generated Code (e.g. by visual studio) 

 2. What are the goals of the code review session?

• Spot out coding errors, e. g.
• Typing mistakes
• Logical error
• Code smell (link)
• Encourage knowledge sharing
• Improve communication skills of team members
• Improve code quality
• Help to enforce coding standard

3. How can we spare time to do code review in a tight schedule project?

• It is true that code review sessions use up developers’ time
• But the time is used up wisely and eventually can save developers’ time.
• i.e. The cost to fix errors at early phases (before code commit) cost a lot lesser than to fix the same error at latter phases.

Recommended Procedures:

1. A developer start working on a development task
2. After coding is completed
3. The developer invite a developer to join a code review session
• Pick someone who is familiar with the areas/dependencies of the new code most as the code reviewer
4. The developer explains the context of the code changes (i.e. the task details) to the code reviewer.
• What are the problems the code to resolve?
• Why a specific solution is picked (assume that there are always more than one solution to solve a specific problem)
5. The developer explain the code changes to the code reviewer (with a diff tool they both preferred)
• The code reviewer can interrupt the developer if the pace is too fast
• The developer should document the comment/feedback from the code reviewer immediately (some of them might be documented in the JIRA task)
• If the developer thinks the feedback is invalid, he should explain the details to the code reviewer
• Feedbacks should be classified into two categories
• Must be fixed before code commit
• Can be fixed after code commit
6. Developer fixed all the “Must be fixed before code commit” issues.
7. Commit the code changes
• Mark the name code reviewer as the “code reviewer” in TFS
• Mark the name of the code reviewer in the JIRA.
8. Handle the “can be fixed after code commit” items.

Tips:

• Drink enough coffee/tea before the code review sessions
• Keep the code review session short (ideally within 15 mins), use a stopwatch to enforce it.
• If the code review session cannot be completed within 15 mins, it means that the code changes are too large for a single code commit
• After a code reviewer give out a comment/feedback, the developer should NOT try to fix the problem immediately, he/she should only record the problem during the code review session. The goal is to make the code review session as fluent as possible.
• If many file changes are similar. Instead of reviewing every single file one by one, to save time, it is better to do sampling and review the same kind of changes once only.

Credit: Thanks to my colleague who send out this :)