Code reviews are an important part of software development. By reviewing each other's code, teams can catch bugs early and maintain consistent code standards across a project. Beyond the technical benefits, code reviews foster collaboration, knowledge sharing, and help identify potential improvements that might be missed in isolation. When done effectively, code reviews not only improve the quality of the codebase, but also enhance team cohesion and individual growth. Code reviews help the developer, the reviewer, and the team as a whole.
Why do we need code review?
Code review plays an essential role in providing an additional layer of scrutiny to the development process. Reviewers help to ensure that all decisions made by the developer are sound, consistent, and aligned with the project's goals. This process not only catches potential errors, but also fosters better practices, offering a broader perspective that strengthens the overall quality of the code.
Learning
Code review is one of the most effective ways to create experts within a development team. By reviewing others’ code, developers gain exposure to different approaches, best practices, and problem-solving techniques that they might not have encountered on their own. Conversely, having one’s own code reviewed provides valuable feedback that challenges assumptions, improves decision-making, and reinforces good habits.
This exchange of knowledge accelerates growth, turning junior developers into seasoned engineers and keeping experienced developers sharp. Over time, this continuous cycle of learning and refinement raises the overall skill level of the team, ensuring that expertise is shared rather than siloed. Code review isn’t just about catching mistakes—it’s a mechanism for building mastery.
Code review is also one of the easiest ways for new developers to become acclimated with a code base. Unlike simply reading through large amounts of unfamiliar code, reviewing an actual change forces developers to trace through the logic in the context of a real problem. This makes the learning process far more engaging and practical, helping new team members understand how different components interact, what patterns the team follows, and where potential pitfalls lie. Over time, this hands-on exposure builds confidence and fluency with the code base much faster than passive observation ever could.
Accountability
One of the key roles of a code reviewer is to hold the developer accountable for meeting project requirements and objectives. While developers are responsible for writing the code, the reviewer ensures that the implementation aligns with the intended goals, functionality, and scope. By carefully examining the code, the reviewer confirms that the developer’s work is on track and that nothing is overlooked, providing an added layer of responsibility to the development process.
Even in a team full of dilligent developers, accountability is very important because often the developer has perverse incentives not to adequately complete the task. For example, the developer may be under pressure to close a particular ticket and in an effort to close it quickly, may miss some requirements.
Safety Net
You can think of a code review as the editing phase of a writer's process. When writing an article, having a second set of eyes ensures clarity, accuracy, and a more polished final product. More importantly, it provides a validation that the content makes sense to readers beyond the author. Similarly, code reviews allow developers to refine their work, ensuring that the final code is not only functional, but also clean, readable, and maintainable. Just as no writer would publish a piece without proofreading, no developer should push code without ensuring that it is as clear and understandable as possible for the entire team.
Code reviews can also be compared to roles where one person acts as a backup to catch potential mistakes. For example, a spotter in weightlifting is there to provide assistance and ensure safety in case the lifter struggles with the weight, just as a code reviewer helps identify issues that might have been missed. The copilot in an airplane supports the pilot by double-checking navigation and procedures, much like a code reviewer ensures that the developer’s approach is sound and the code is free of errors. Lastly, a safety net in acrobatics catches the performer if they fall, much like a code review catches overlooked mistakes before they impact the final product. In all of these cases, the secondary party acts as a safeguard, helping to ensure everything runs smoothly and safely.
People Involved in a Code Review
There are several actors who may be involved in a code review, which will be described below.
Developer
The developer, sometimes referred to as the author, is the person who wrote the code that is under review.
Reviewer(s)
The reviewer (of which there can be more than one), is the person who is tasked with reviewing and testing the developer's code.
Tester(s)
The tester is the person who is tasked with testing the code change. Sometimes a tester is not included as part of the process, and testing responsiblities are limited to the reviewer.
Code Review Tools
The most common, and classic, tool for code reviews is Github, which provides a structured workflow for proposing, reviewing, and merging changes through pull requests (PRs). Although many other tools exist, this article will focus on Github tooling due to its prevalence.
Preparing a Code Review (as Developer)
Once a code change has been completed, the developer's next responsibility is to prepare the code review. This preparation is important for setting the code reviewers up for success. The developer should strive to provide as much information as necessary for a code reviewer to understand the change and be able to complete a quality code review.
Some elements to include in a good code review may include:
Description of the change
Don't leave it ot the code reviewer to try to decipher what is being changed and why. Provide some background information. Tell the code reviewer why you are making this change. And also tell the code reviewer how you are making this change.
Example: Fixing a bug in a date formatter
This change fixes an issue where dates in the report export feature were displayed in UTC instead of the user's local timezone. The problem was caused by the backend returning dates without timezone information, which the frontend interpreted as UTC. To fix this, I updated the backend to include timezone offsets when sending date values and adjusted the frontend to correctly parse and display them in the user's local timezone.
The description is important because it provides useful context to the reviewer for when they start the code review. They have a general idea of what they're looking at and what to test for.
A description is also important for whenever someone goes back to perform a Root Cause Analysis (RCA). Perhaps years into the future, developers will look at this code and wonder why it was written a certain way. Perhaps they'd like to change it to fulfill new requirements, but are worried about breaking some obscure workflow that they are unfamiliar with. Those developers can go back and find the Pull Request (PR) that originally introduced the code. If there is sufficient context and information, they should be able to answer any questions that they have.
Screenshots
If there is a visual element to the change, include a screenshot or a video. A picture is worth a thousand words, and sometimes a quick screenshot is all that's needed to quickly align the code reviewer on what they should expect to see when they test the code.
Test Instructions
Tell the code reviewer how to actually test the code. Don't assume that the code reviewer is as intimately familiar with this workflow as you, the developer, are. The code reviewer may not have ever encountered this workflow before. The code reviewer could even be new to the team. Providing steps for how to actually test the change makes the code review faster and more efficient by not forcing the code reviewer to spend time figuring out how to test it.
Tips:
- Inlcude any settings, configuration, or other build that might be necessary to test the change.
- Include any sample data that might be helpful in testing the change
Why Else is Documentation Important?
Documenting a code review well is not just important for ensuring the code reviewer is best equipped to perform a quality code review. But this documentation also serves as valuable information in the event that a PR is ever revisited at some point in the future. Sometimes, bugs are discovered months or years after they have been created. Serious issues may require some investigation into what happened. Being able to review a PR's documentation provides hints and important context surrounding the decisions made at that time. The same is also true for when a piece of functionality is considering being changed. Being able to review the context around the original functionality can inform everyone on whether or not the proposed change would reintroduce any issues that had already been discovered.
Starting a Code Review (as a reviewer)
Code review is a back-and-forth process in which a code reviewer will point out potential issues and ask questions, and then pass it back to the developer who will address or reply to all of those comments and pass it back to the reviewer. The process will then repeat until both parties are satisfied with the quality of the code change.
As a reviewer, there are typically at least two phases to every code review.
- Static code review
- Testing
Phase 1: Static Code Review
The first thing a reviewer should do is review the diff of the code changes. This will help the reviewer to understand the modules that are being modified as part of this change and will provide ideas about what to eventually test.
It's even ok for the reviewer to write out a few notes as they are reading through the diff. For example, a change in one file may look suspicious, so perhaps the reviewer wants to make a note about that to come back to it after they have finished reviewing all of the files.
During the static code review phase, the reviewer can point out potential coding style issues, such as code that doesn't meet the style guidelines of the team. There may be more serious code-related issues, such as branches of code that are not being covered or handled properly.
NOTE
Nitpicking should be kept to a minimum as it takes up time and diminishes the autonomy of the developer. However, from the developer's perspective, please keep in mind that although you are the developer of this code, ultimately the code is "owned" by the team; so be open to suggestions that might be more aligned with the rest of the team.
After the initial static code review has completed, pass the PR back to the developer to have them respond to or address all open issues.
TIP
If you are using Github for your code review, use the "Start Review" button to make all of your comments. What this will do is prevent the developer from being notified about every individual comment. This eliminates the inefficiency of the developer not being sure whether you have actually finished the code review or not and avoids chaos of a developer making code changes while a reviewer is still actively reviewing them.
Additionally, as a reviewer, you may realize that some of your early comments are no longer applicable after you have read through more of the code. In that case, you may wish to go back and delete some of those comments. By using "Start Review", you allow yourself the ability to write or remove comments, uninterrupted, until you have fully completed your review.
Phase 2: Functional Testing
The reason for the two phase approach in Code Review is because if any changes are made to the code as a result of Phase 1, then any testing that was performed is now invalidated. When code changes, the state of the entire system changes. Any testing that was done on a previous state would need to be redone on a new state to ensure that there were no regressions. So, it's a waste of time to being testing if there is a possibility that the developer will make further changes.
Once the developer passes the PR back to you after making any necessary Phase 1 changes, now you can begin functional testing. Put yourself into the shoes of a user and test the functionality of the change. Try to think of variations of the workflow that might cause the code change to break. Use the added knowledge you gained from reviewing the code itself to identify areas where you might be able to break it. For example, maybe you found a function that isn't adqueately guarding for null
or invalid inputs. Try to see if you can actually get those invalid inputs into that function. If you can, write up an issue and explain to the developer what happens when you perform that workflow.
Document all functional issues that you find. When you find these issues, it's best to write them up in the same way that you would write a bug ticket. In other words, include the steps to reproduce the issue, what your expected behavior is, and screenshots to help clarify the issue.
In the same way that you want the developer to make it easy for you to test their code, you should make it easy for the developer to understand and fix the issue that you have documented. If you don't include the "expected behavior," for example, then sometimes an issue can be misinterpreted and won't be addressed in the way that you had in mind.
TIP
As a code reviewer and tester, it is your job to identify deficiencies in the new code. If there are any issues, you should consider it your mission to find them. In fact, it's your responsibility. Do whatever you can to find those issues because it's better that you find them, than an end-user does.
Pull Down the Branch
It is absolutely essential that you pull down the developer's branch when doing code review. Reviewing the diff of the PR is not sufficient.
First, if you don't pull down the branch, then you may not be able to actually test the code. You may not be able to verify that the code actually compiles and passes all of the linting and other checks it needs to pass. You may not be able to run the unit tests and verify they all pass.
Many teams now have automated checks for linting and unit tests that will prevent a PR from advancing if they do not pass. Some teams may have the capability to test a branch of code without actually pulling and running the code. If so, great! Utilize those tools as they save a lot of time and add a lot of wonderful safeguards. However, even with these tools, you will still have to download the code...
Because, suppose a developer deletes a global constant. Or suppose they rename a function. Any of these actions would require all references to that constants/function/etc. to be updated. Hopefully the developer has done a quality code search and found all the places it was referenced. But it's your job as the reviewer to double-check it! And please note that not all programming languages are strict about catching these kinds of issues during the build process. While some modern IDEs or linters may flag missing or renamed references, they might not catch every instance, especially if the code is dynamically generated or relies on less obvious patterns.
Please Test the Code!
As one final reminder: Please test the code you are reviewing!
Ensuring that the code actually works is the absolute most important function of a code review.
Testing code is extremely important, yet it requires more effort than simply pointing out readability issues like naming conventions or formatting. Running the code, testing various scenarios, and ensuring correcntess demands time, attention, and sometimes more advanced setup. Unfortunately, many reviewers either don't have the time or don't want to invest the necessary effort, so they default to giving feedback on subjective elements like style, which are much easier to critique.
Readability, while important, is highly subjective. Unlike functional correctness, which requires understanding the underlying business logic and testing how the code performs in real scenarios, readability is often opinion-based. Because of this, many reviewers feel comfortable offering opinions on these items without needing to deeply engage with the core functionality of the code. It is a low-barrier way to contribute to the review, but often at the cost of missing more critical issues. This tendency to focus on the "easier" comments can lead to a less effective review process, where the focus shifts from verifying correctness to nitpicking style. It's also a wasted opportunity to catch bugs in the system before they make it into production.
Reviewing is Not a Rubber Stamp
Code review should never be treated as a mere formality or a rubber stamp to approve changes. It’s a crucial part of ensuring the quality, functionality, and maintainability of the codebase. A reviewer’s role is to actively engage with the code, ask questions, and challenge decisions where necessary. Simply glancing over a pull request and clicking "approve" without thoroughly testing the code, understanding its impact, or considering the broader context of the project undermines the purpose of the review process.
In some cases, developers may feel pressured to quickly approve code to meet productivity metrics or gaming the statistics, like review completion times or the number of pull requests approved. This kind of "rubber-stamping" creates a false sense of progress and can lead to overlooked errors, poor-quality code, and missed opportunities for improvement. Effective code reviews should focus on quality, not quantity, and reviewing with genuine attention to detail ensures that the team produces high-quality, maintainable software in the long run.
Responding to Code Reviews (as a developer)
As the developer, it is your responsibility to address or respond to all issues written up by your reviewers. Some of those issues may not be legitimate issues. For example, they may be expected behavior. That's ok, simply reply to the issue and say so.
If a reviewer writes up a legitimate issue that requires a code change, make the code change and respond back to the issue indicating that you have fixed it and adding any additional context that may be useful about that issue. For example, "I fixed the issue by doing ...".
It's often useful to write a separate commit for every individual issue, particularly when there are a lot of issues or a lot of reviewers. For example, if 100 issues are written up about a particularly large PR, and they are all fixed in one giant commit, it can be very difficult for reviewers to go in and verify each of the individual fixes. On the other hand, having individual commits for each individual fix makes it easy to link the reviewer to the exact change made to address a specific issue.
TIP
Here are a few ways in which you might respond to a code review comment that has been addressed:
- 👍
- Fixed
- I have addressed this issue by changing...
- This is a good suggestion, but I don't think we should do it because...
- This is out of scope of this PR, so I have created a ticket for it: {ticket}
Reviewing Patches to a Code Review (as a reviewer)
As you write issues to the developer, you must also verify that the fix is made in a satisfactory manner. If you have written up an issue and the developer indicates that they have fixed it, you must follow-up and ensure the developer has actually and adequately fixed the issue.
There are times when there is a miscommunication and the developer may not have understood the issue that was written up by the reviewer. In that case, the developer may have fixed the wrong thing.
That's why it is up to the original reviewer who wrote up the issue to verify that the fix was made.
This process may also utilize the Two Phase approach. First, the reviewer can review the diffs of the intermediate commits that were made to address the issue. And if those look good, then the reviewer can move on to testing and verifying that the functionality itself was fixed.
Resolving Comments
It is the reviewer's responsibility to resolve all of the comments that they write up. Resolving the comment closes the loop on an issue. That loop being: the reviewer reports the issue, the developer fixes the issue, and the reviewer verifies the fix. Resolving the comment cleans up the PR and makes it easier to visualize how many outstanding issues still remain.
Ideally, a code review should not advance if there are any unresolved comments on it. Because if so, that means that a reviewer has identified a problem with the code and we should not proceed until that problem has been addressed. Even if the problem is not "fixed," perhaps it should be written up as a ticket to address in the future.
Writing Up Separate Tickets
Sometimes, throughout the process of testing a PR, a reviewer will find issues that are unrelated to the PR they are reviewing. Or sometimes, they may believe that an issue is related to the PR, but in fact, it is out of scope.
It's perfectly ok to write these issues up in the PR. The expectation is that the developer will investigate them and respond back to them stating that they are unrelated or out of scope.
However, out of scope or not, these are still issues that may need to be fixed. Don't let them get lost inside the PR. Instead, convert them into tickets that can be reviewed and prioritized independent of the PR. That helps to ensure that the application as a whole is being maintained with high quality.
If you are reviewing a PR and you find an issue that you know is unrelated, please don't just let it go. Eventually someone will find the issue and that someone may be a user. If you notice something is wrong, make sure to address it before it gets to that point.
Types of Issues
There are a variety of issue types that might come up during a code review. Below I have made a list of some possible categories of issues:
- Crash
- Functional Problem
- Performance
- Security
- Visual Glitch
- Code Style
- I18N
- Question
FAQ
How do I handle nitpick comments?
As a developer: As a developer, nitpick comments can sometimes be frustrating. It's tempting to view these as damaging your independence and autonomy as a developer, for no legitimate functional reason. But try to consider that once the code is merged, it is no longer "my" code, but rather it's "our" code. And while a segment of code may make perfect sense to me, it may not be as clear to another reader. My code reviewer is one of those readers, and if they are having trouble understanding it, then maybe I haven't written the most clear code.
That doesn't mean to give in to every coding style issue that gets written. It's best to choose your battles. If it's important to you, say so. If you don't care so much, consider changing it, because it was important enough to the reviewer to write it up.
As a reviewer: As a reviewer, try to avoid nitpick comments unless they are significant enough to comment on. It helps to have a reason for the change beyond just personal preference. Recognize that comments like this are ultimately subjective and that you and the developer may not always agree on that subjectivity. Nitpicking comments can add overhead to code reviews that take up time, so we should be sure that those time investments are truly worth it.
Often, nitpicking comments are hypothetical in nature—suggesting changes that may not have a concrete impact on the application at the current time. Before making such a comment, ask yourself: Does this change meaningfully improve the code, or is it just my personal preference? If the answer leans toward preference, consider whether it's worth mentioning. When in doubt, framing suggestions as optional and explaining their rationale can help keep the discussion productive. The goal is to foster collaboration, not to enforce personal coding styles at the expense of efficiency and morale.
I often challenge myself to find an actual functional problem with the code before writing up comments like this. For example, a reviewer might suggest replacing a ==
comparison with ===
in a conditional statement. At first glance, this might seem like a minor nitpick. However, if the comparison involves values of different types (say, strings and numbers), then the ==
could cause unexpected behavior. If you can find an actual bug and your comment provides a tangible benefit, then we can ensure that feedback is both constructive and worth the developer's time to consider.
Lastly, if you are going to leave a nitpick comment, please make sure that it actually works. How many times has someone told you to change the code to something and then you try it and the syntax is illegal? For example:
Change it to this
MyData
. (But actually, it has to beMyData<T>
)
Is Paired Programming A Substitute for Code Review?
Paired programming is an excellent tool for writing code and having it reviewed simultaneously. It's also beneficial for developers to observe how other developers work. Often times, you can pick up useful tips that you wouldn't have otherwise known about. For example, you might learn some new and better ways to debug code by watching how another developer does it.
Paired programming can be considered a "code review" in that it's done in parallel to the development itself. After a paired programming session, there's nothing wrong with approving a PR as soon as it's done, because the code has indeed been reviewed by the person doing the paired programming.
In some situations, paired programming can save a large amount of time. Typically, when a PR is passed to a reviewer, the reviewer needs to spend some time understanding how the code works and what the change is doing. The developer has already invested the time to do this while they were doing their development. By pair programming, we avoid doing some of this twice.
Testing Code Requires More Effort Than Pointing Out Readability Issues Running the code, testing different scenarios, and verifying correctness takes extra time and effort. Many reviewers either don’t have the time or don’t want to put in the effort, so they default to commenting on naming, formatting, and structure.
Readability is Subjective, So Everyone Feels Qualified to Give an Opinion
- Unlike functional correctness, which requires understanding the business logic and testing the changes, readability is opinion-based and easy to critique.
- People naturally want to contribute something to a review, and readability is a low-barrier way to do so.