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 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 boost his confidence. This is the clear sign that engineer is matured, technically strong and experienced enough.
- During the review, whole lot of implementation details are discussed with the owner of 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 new language quickly if you are new to it.
The owners of the code needs to get his code review from at least two colleagues. The code reviewers has to keep below in mind while reviewing.
- 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
- 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 licenced.
- See if timeout are incorporated in IO path, should not hang the IO.
- If developer is using open source libraries ask how stable they are.