For many programmers code reviewing is a mere chore. They know that on paper it is an important part of the development process and it is required by the people from quality assurance, but they feel it is less important than actual programming. Consequently, code reviews are often postponed and then, finally, done in a rush. This becomes a self-fulfilling prophecy because a bad code review is indeed a waste of time. In this article, I will give 10 tips to make your code reviews more valuable and show why code reviewing is just as important as actual coding.
10 Tips for Valuable Code Reviewing
1. Review what is missing
The most common mistake I see when people are code reviewing is that they only review the code that was added and not the code that might be missing. Of course, you review the code presented to you in the merge request. If there are flaws in there, you will likely spot them. The code you do not see, however, you have to actively look for and think about. It requires a good understanding of the changes in a larger context. Maybe all changes that were made are well executed, but one file that should have been updated was not. All added code can be written off as production-ready, but once it will be deployed it would still produce a bug!
2. Check out the code
For large changesets that require reviewing, it can be easier to just check out the branch containing the changes on your machine. This gives you all the navigations and relations in the code that your IDEA provides and allows you to naturally follow the flows that were added or modified.
3. Guard consistency
Every developer has their own writing style. I feel that up to a certain level this can be seen as artistic freedom and it is as harmless as a signature under the code. However, consistency in a codebase really improves readability for new developers acquainting themselves with it. Therefore, while reviewing, try to find a balance between accepting individual styles and guarding the consistency between this new changeset and the rest of the codebase.
4. Learn from the code you review
Code reviewing is not just putting red lines under code you don't agree with. It is also a possibility to learn new language features, constructs, patterns, and libraries. Moreover, when you really try to understand the code you are reviewing, you are pro-actively gaining knowledge on this new changeset, spreading the code ownership of it.
5. Review dependencies
It is only natural to focus on the core changes of a merge request. Things like use cases, domain objects, and services. Often, however, issues can be found in the "shadows" of the changeset. One of these dark places is the dependency overview. Are all newly introduced dependencies really necessary? Are the versions in line with versions of other applications? Are there security risks in the versions introduced in this merge request? All these checks can save time and problems in the future.
6. Review configuration
Another place that is easily overlooked is the configuration that was changed. It is 'not actual code' and when you are in a hurry to complete a code review as fast as possible, you might be tempted to skip it. Nevertheless, configuration is exceptionally tricky and should be carefully reviewed. When it is written separately for testing, staging, and production environments, a typo in a production configuration file can go unnoticed during testing and will likely only be found during code review or once it's already deployed.
7. Check the coverage
Even when there are thresholds for test coverage it can be good practice to check whether no critical parts remain uncovered by tests. A threshold of 80% coverage does not mean that a developer can stop writing tests once that threshold is reached. All important business logic and classes should be covered, even though that might mean 90% of the code has to be covered.
8. Review comments and documentation
Think ahead when reviewing a merge request. Will developers maintaining this code in the future understand this code? If not, suggest adding comments to parts that are not self-explanatory. Also, consider whether some higher-level documentation should be written about the code in this merge request or if existing documentation should be updated with regard to the new changes.
9. Comment what is good
Do not be afraid to also remark what you find good in a changeset. When there are only a handful of suggestions to be made, all other parts are good and that is worth mentioning. There is no reason to only pinpoint the negative parts of a merge request. That makes the reviewing process even more unpleasant for both parties whereas a simple compliment on a clever implementation can make someone's day.
10. Review in advance
Finally, you can save a lot of review time and effort by pair programming. Discussing design decisions early in the development phase prevents a lot of refactoring when compared to revisiting core design decisions at the end of the development during a code review.
Conclusion
Code reviewing can be much more than quickly scanning someone else's work. If done properly and with care, it can distribute knowledge between team members, improve future maintainability of a codebase, and prevent bugs. Improve your code reviewing by:
- Looking for what's missing (code, docs, tests), on top of what's wrong
- Learn from the code you read
- Give constructive and positive feedback
So the next time a team member asks for a review, do not stay quiet but raise your hand and try these tips. Good luck!