Most of the cases I have seen engineers are not doing efficient reviews because it’s not really their code (someone’s code.) or no time to do. The mindset is very important during code review.
Importance of Code Review:
- This saves a lot of time in fixing bugs so that we can release the product in time.
- This is the time engineers learn/teach effective programming practices, language semantics/constructs, new libraries, and algorithms.
- If an engineer finds bugs during the code review time, it boosts his confidence. This is the clear sign that engineer is matured, technically strong and experienced enough.
- During the review, a whole lot of implementation details are discussed with the owner of the code, for other members, it’s a quick way to learn what other team members are doing and what is going into the product.
- You can learn a new language quickly if you are new to it.
The owners of the code need to get his code reviewed from at least two colleagues. The code reviewers have to keep below in mind while reviewing. If the changes are huge, a code walk of changes needs to be done by the owner.
- Functionality is implemented as discussed.
- Handle Failure cases all over the code.
- Have enough comments in the code.
- Check for proper copyright headers in the code.
- Does the code scales for future requirements.
- Product does not break during Upgrade and Downgrade scenarios
- Does it have all the stats covered for debuggability?
- Logging is sufficient to pinpoint the issue in the code.
- Adhering to language Coding guidelines
- The performance will not be degraded.
- Memory allocations and frees are handled properly.
- In case, the implementation involves multiple threads, make sure code is not making CPUs 100% utilized. Also look for race conditions.
- If you think the new code is too risky, ask for more unit/component test coverage.
- Check if the security is compromised as part of the new code.
- Make sure the developers not copying the code from sensible licensed public code like GPL licensed.
- See if time outs are incorporated in IO path so that I will not hang.
- If the developer is using open source libraries ask how stable they are and attributions are listed in release notes.