choong pw

Nowadays, a good software development company that works in a team typically will establish some sort of process for code review. One of the most commonly used examples is creating a Pull-Request (PR). Commonly used in Github and open-source projects. Therefore, it is good to master these best practices to make your life as a reviewer and reviewee easier.

What Code Review?

Mistakes tend to happen. Having an extra eye to inspect the code helps to catch bugs, improve logic or performance of the code, or just general code and syntax styling for readability.

Regardless, the whole idea is to improve the software development quality by asserting the team to write better code and software. It also served as a platform to close the gap and knowledge exchange between senior and junior engineers when reviewing the code.

Think of it as an art than science. Having a good code review practice, helps the reviewer read and understand the PR effectively. Thus, saving precious time and sanity trying to decipher the code.

Tips

Here are a few tips generally what I think is good. Also, credit to Clément Mihailescu for some thoughts behind this post.

#1 Commit

Reduce the line of code for a single pull request. If that’s the nature of the pull request solution. Try splitting it into multiple commits.  Generally, anywhere between 10 to 100 lines is clean and simple enough for the reviewer to read through it with ease.

Also, if you’re working on linting or refactoring. Separate the commit from your implementation. This helps tremendously instead of trying to screen through all the “not so” important changes.

Make sure you have done proper due diligence before submitting it for review. If the code is integrated with the CI process, ensure that CI checks are passed. Do not commit buggy and unfinished code.

#2 Description

Provide enough description and context. It can be through PR’s comments or inline comments on the code. Anywhere you see fit and useful to provide context to help people understand the reason behind the implementation. It can just be a non-permanent fix, mention it.

If your teamwork using some sort of ticketing system. Link them(the ticket) to the commit or description. Take advantage of tools that able to link them together. You’ll be grateful when shit happens and is able to trace back to the changes on the code level, and read through the discussion.

Provide screenshots before and after the change if you’re working on UI-related tasks. How much time can be saved just from looking at the picture, instead of having the reviewer recompile just to validate the correctness.

# 3 Reviewing PR

If you’re the reviewer. Be clear and explicit about what you want the reviewee to do. It can be just a suggestion, or you don’t strongly agree but is acceptable implementation, or just asking for clarity. Mention it! and don’t leave the sentence hanging. People will get confuse.

Also, do not leave the PR hanging after providing comments. Is either you approve or not approve. If you do not approve it, provide a reason for it as well. This is a means of indirect communication (especially in a remote working environment), you waiting for me or the other way without knowing it has already done.

At times, we might be focusing on other things, and don’t have the real-time response to review again for the fix, right? Establish an explicit standard with your team. Such that, what does it means when you approve and provided comment to fix the PR. Have faith in them to fix it. Or, might just be better to discuss it offline, mention it.

Here is a short code review checklist pay attention to when conducting a code review. The details can be found here.

• Code readability
• Security
• Bugs, edge cases
• Errors processing
• Logging
• Input data validation
• Stability
• Compliance with requirements
• Reverse compatibility
• Code smells