Code reviews are an essential part of software development, as they help improve the quality, readability, and maintainability of the code. However, code reviews can also be a source of frustration, confusion, and errors if they are not done properly. In this article, we will discuss some of the most common code review mistakes and how to avoid them.
A code review should have a clear and specific goal, such as finding bugs, improving performance, enforcing standards, or adding features. Without a clear goal, the code review can become unfocused, inconsistent, or irrelevant. To avoid this mistake, you should define the goal of the code review before starting it, and communicate it to the reviewer and the author. You should also use a checklist or a template to guide the code review process and ensure that the goal is met.
A clear goal is essential for an effective code review. To facilitate an efficient code-review process, the following practice could be helpful: 1. Create checklists or templates and ensure all aspects relevant to the goal are covered. 2. Clarify expectations and fine-tune the review's direction. 3. Use code review tools such as GitHub that allow tagging comments with the specific goal they address, improving tracking and follow-through.
The goal of a review heavily depends on the content of the PR. What was the intention in the first place? writing a new feature - look for obvious bugs, bottle necks and consistency. Was this a bugfix? Are there tests prooving that the bug was fixed? Is it only a short term solution aka fixing symptoms instead of treating the illness? Was it a POC or performance investigation? Where is the data and where are the findings. Based on this, we have checklists in place that help to investigate upon each topic further.
A code review should provide enough context for the reviewer to understand the code and its purpose. Without enough context, the reviewer might miss important details, make wrong assumptions, or ask unnecessary questions. To avoid this mistake, you should provide a brief summary of the code changes, the motivation behind them, the expected behavior, and any relevant information. You should also link the code to the corresponding issue, task, or user story, and use descriptive commit messages and comments.
Think of context as a map for the reviewer. Without it, they are navigating potentially complex code without a clear sense of the destination. This leads to several problems, such as wasted time, inadequate review, and potential misunderstanding. To remedy this: 1. Focus on explaining the motivation behind the changes instead of just describing what was done. 2. Issue trackers, task boards, and user stories offer valuable context. Therefore, one must link directly to them. 3. Descriptive variable names, well-written comments, and informative commit messages provide ongoing context beyond the initial code review. 4. A quick conversation with the reviewer before they dive in can be constructive.
A code review should be respectful and constructive, as it is a form of feedback and collaboration. Without respect and constructiveness, the code review can become hostile, defensive, or personal. To avoid this mistake, you should use polite and professional language, avoid sarcasm, insults, or jokes, and focus on the code, not the person. You should also give positive feedback, praise good practices, and suggest improvements, not demands.
A code review should follow the coding standards of the project, such as naming conventions, formatting rules, documentation guidelines, or best practices. Without following the coding standards, the code review can become inconsistent, confusing, or messy. To avoid this mistake, you should familiarize yourself with the coding standards of the project, and use tools such as linters, formatters, or code analyzers to check and enforce them. You should also avoid nitpicking or arguing about minor or subjective issues that do not affect the functionality or quality of the code.
A code review should test the code before submitting it for review, as well as after applying any changes. Without testing the code, the code review can introduce bugs, errors, or regressions that might affect the functionality or performance of the code. To avoid this mistake, you should write and run unit tests, integration tests, or end-to-end tests to verify the functionality and behavior of the code. You should also use tools such as code coverage, code quality, or code complexity to measure and improve the testability and maintainability of the code.
One common mistake in code reviews is not testing the code. I recall a time when a seemingly harmless change was merged without testing. It led to a critical bug in production. Always ensure to test the code, even if the changes appear minor. It's a lesson I've learned the hard way.
Even though it is often frowned upon, code coverage on new code from e.g. sonar qube really helps adress this to a certain degree, because developers are forces to think about testability. Of course a reviewer needs to check for the content of test if they exist only for the sake of the coverage tool or if they actually hold value to the code base.
A code review should address the feedback from the reviewer, as it is a way of learning and improving. Without addressing the feedback, the code review can become ineffective, incomplete, or ignored. To avoid this mistake, you should read and understand the feedback, ask for clarification or explanation if needed, and acknowledge or thank the reviewer. You should also make the necessary changes, explain your reasoning, and update the code review status.