How to Provide Effective Feedback based on Code Review Type
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…