Type of code reviews
There are several types of code reviews and you may want to adjust your style depending on the type of review you are undertaking:
- Bug fixes
- Other: script to automate a recurring task, github template, …
Features are more likely to come to you in a series of code reviews. A good practice for the person requesting the review is to include you in the design of the feature (i.e. asking for advice, reviewing, co-authoring a design doc) The level of prior involvement will depend on the size of the feature.
As you are reviewing the feature you should ask the following design questions:
- Do the interactions of various pieces of code make sense?
- Does this change belong in this codebase, or in a library?
- Does it integrate well with the rest of the system?
Discuss and iterate on overall design before discussing details in the code. Then review the functionality of the piece of code.
Once the code is out for review it is reasonable to expect that it works correctly. The reviewer brings a fresh perspective:
- What are the edge cases?
- How could a user break this code?
- Is there a potential for concurrency problems?
- Is there prior knowledge or art that I know about this portion of the code base?
- Is this code “too complex”? Can I understand the code quickly or are there ways it can be simplified for the future.
Bugs fixes tend to be much smaller and more focused. They may also be on a timeline. This is where the reviewer’s prior knowledge of the code may be the most useful way they can contribute. It may also be useful to try to break the code, to ensure it really fixes the issue and does not introduce another bug.
As a reviewer you should stay focused on the goal of the review and table discussion of deeper changes, or style changes for future. If needed, open another issue to track other changes rather than delaying a bug fix.
This is the case where you may want to give a LGTM (looks good to me) upfront, depending on the purpose of the review. For example, if it is a new issue template, you may want to point out spelling mistakes, items you may find useful and give them approval, so they don’t need a second round of communication with you.
The goal of all of these types of reviews is to improve the overall code health and share knowledge. Reviewers can also learn by reviewing the code others have written.
Remember it doesn’t have to be perfect and you can use a TODO or similar to iterate in another review or capture an idea for the future.
This article is a series on how to be an effective code reviewer. Please see my previous article that provides background on Code Reviews and Their Benefits. The other articles in the series are: