A Checklist For Code Reviews
Code reviews are actually easy, you have to make sure that the code that is checked-in is code that you will be able to maintain later on, that is the only purpose for which the process exists. The only goal is to keep the codebase in good shape over long periods of time.
The amount of changes in a code review is as many lines or as few lines as the feature requires to be whole but there should be rules to how often you ask for an intermediary review while you are writing the entire thing.
If you keep running into coding styling issues, forgotten print statements and TODO comments, please make yourself a favor a lint those things away.
Reviewing incremental code changes gives you the opportunity to learn about a project, to be part of the decisions that lead to the latest version of it and gradually understand what is going on. So, reading code reviews is as good of an exercise for the one who reads it as it is for the one who is being reviewed. Make use of Draft Code Reviews in order to see incremental changes take place and have the ability to make small corrections with time to spare rather than big reviews two days before the feature has to be released.
There is always something to be learnt by reading the concrete implementation of the feature that might have been overlooked beforehand. Also, when working directly with code, a lot of possibilities open up because you are actively thinking on the problem at hand which opens many possible solutions to the same problem or that could make other features easier to implement. Maybe while you are reading a piece of code you see something or a concept or a solution that might apply to another project that you are working on.
For the reason stated above among many others, the Code Reviewer doesn’t necessarily have to be a person that is actively working on the project or not even work on the project at all but it should be a person with strong technical skills and, above all else, good judgement.
In order to ensure this, I use the following checklist.
- Does it solve the problem?
- Is this a good solution?
- Is the code readable?
- Do I understand the problem they are solving?
- Do I understand the trade-offs they are making?
- Could I work with the codebase if I had to?
- Is the code easy to refactor for new requirements?