What makes for a great code review?
Here is one of our favorite responses to get started:
“What makes for a great code review? The answer is the "Holy Grail" we are looking for. Almost every bug discovered in the production environment in last two years or so was due to mistaken or omitted code review. The requirements expressed by a natural language are often ambiguous, do not specify required behavior of the software in margin conditions and so on. You may have many tools to support continuous integration, code quality and coding standards conformance, but you must review the whole development process from its beginning, i. e. from the correct understanding of requirements to its materialization in the code. Otherwise you get "nice and conforming" code, that does something different than what has been required.”
We have organized the responses below by key themes to make reading through a little easier. Enjoy!
A Learning Culture
“First, fostering an atmosphere of learning and growth. There needs to be a common perception of code reviews being an opportunity to grow as a developer, and to have a safety net, rather than an opportunity for criticism and admonishment. Second, sharing the "why" in addition to identifying the "what" that is the problem or the "how" to fix it.”
“Psychological safety is crucial. People need to be able to accept and deliver both constructive praise and constructive criticism during a code review. Developers need to be willing to accept feedback from others, but also have the confidence to deliver feedback to others. Without full, yet professional, honesty and transparency, the efficacy of code reviews, and team collaboration as a whole, are diminished.”
“The right spirit in the team: Reviews are seen as a chance for improvement, not as criticism. There is a basic understanding about how code should look like, and why that is important. The right tool for the job: One that shows changes, lets you think about the changes in your own time, and keeps comments/defects in the right place, even after several uploads of changes.”
“The human touch makes for a great code review. People write code under a set of circumstances - time to hit deadlines, odd requirements, language barriers, and time barriers. I work in a 100% distributed team and in open source. A great code review considers the factors outside of the code to help deliver better code.”
“Culture of giving and accepting feedback (for senior people as well). Senior developers that make it one of their priorities to review junior's code. Reviewer should understand the overall goals of what the change is trying to accomplish as well as comparing design patterns between different developers to make sure they are similar.”
“A great code review is one where the developers are open to constructive criticism and have been adhering to any existing development standards. While everyone has slightly different ways of doing things, being open to criticism or suggestions to improve their work or to bring it more in-line with others can help reduce the amount of time required for reviews. The same goes for adhering to standards. The closer code is to existing standards the less guessing people need to do regarding variable names, structure, etc.”
“In my opinion, whether a coder or a reviewer, understanding that you are not perfect. There cannot be any room for big egos on either side. Strong onus of reviewer to make the review successful by understanding the humane aspect of development. Don't be authoritative. Be adjusting and understanding. Try engaging team in review process to make others understand the functionality and setting up the expectation and standards for future reviewer. Avoid offline reviews. It is always good to understand the developer's perspective while doing the review.”
- "Point out what could be improved.
- Explain why the submitted code is less than ideal, and offer suggestions on how to improve it without doing it for them.
- Provide link to detailed article explaining the how and, more importantly, the why.
- Write the above using encouraging and respectful language.”
“Getting value from code review is all about fostering a culture of open questions delivered in a polite and professional format. If reviewers are reluctant to ask questions, it's a complete waste of time. On the other hand, if the author experiences the review process as a personal crucible, it'll lead to team friction. It's all about carefully establishing code review as a team cooperation effort where everyone is interested in honest feedback to improve their code and their coding practices. It takes a while to establish, and it takes effort to maintain, but it's essential to making code review worth anything. If you're not asking questions, you're not developing an understanding of the code and the problems it's solving.”
“The best code reviews are honest and constructive. Sometimes you don't know why something was coded a certain way, which gives you a chance to ask and learn. It also gives you an opportunity to help someone else out. And sometimes the code is great and you can let them know that too.”
“Looking at the high-level construction of the code. Asking thoughtful questions. A variety of different people doing the review (at least 2).”
“Ruthless dedication to best possible value for the client, company & team. No compromise due to any kind of emotional feelings or fear of hurting someone's ego. A fine-grain story, leads to a contained pull request, which leads to a focused, effortless and efficient code-review. A good code review is therefore a product of the whole team being engaged throughout the whole development process.”
Fast Feedback Loops
“Small incremental code changes with a fast turnover and Continuous Integration. You don't want to wait too long to get your code reviewed and get it up and running in some shared environment. Having a good culture focused around code reviews helps too, people need to be educated on how to take and receive constructive feedback and they need to understand the advantages code review brings to them.”
“Reviewing small chunks of code with proper unit tests written greatly improve the code review process. Great code reviews will tell you why you want to make the changes being suggested, not just that you should make the changes. Any company coding standards should be well documented and frequently seen mistakes or broken standards should be addressed to the whole team instead of just kept between the developers working on the review.”
Attention to Code Maintainability
“Don't just focus on coding standards but make sure the code is readable and easy to reason about for future devs. Look for design flaws more than simple bugs which often can be tracked down using static code analysis tools. BTW, use those (e.g. Sonar) while doing a code review so you don't have to figure out the obvious.”
“The review that finds, and points out all mistakes and also guides on how can they be mitigated or made harder to repeat. This not only applies to bugs but also to improving readability of the code being written. Code is for humans and machines and it should be written as such. Make it hard to read or "rationalize" by any of both is one step closer for disaster.”
“The person doing the review needs to understand the complexities of the code and what the expectations are around code review. When they don't know that, code reviews are basically useless.”
“Meaningful checklists. I used to be annoyed by code review checklists until I found myself creating my own checklists. That's when I realized that the problem was the institutional checklists had little relevance. A meaningful checklist needs to be developed by the team by reflecting on the types of bugs most often found in review, and lessons learned from past bugs found after release.”
- needs to explain clearly the goal of the change and how the change achieves the goal
- should write the change in a way that maximizes the chance that the company's diff tool of choice will focus on substantive changes
- need to seriously engage with the code
- should avoid commenting on extraneous issues (where the definition of "extraneous" is company or dept or even team-specific)
- need to comment respectfully”
“Ensuring that each task/story has been succinctly written so as to capture all requirements clearly. This allows for a clearer path to unit testing and functional tests, also makes it easier for code reviewer to understand what is trying to be achieved meaning that the review can be done more quickly.”
Proper Context for Reviews
“Having a review done by people who know what to look for and have a reasonable expectation of understanding the code and its impact. A number of data points collected by a number tools are required for such a complete picture and code alone is not enough. Understanding how the code effects the process as a whole is an important part that would provide more of the context needed to fully assess.”
“Context. Code reviews are much more useful if the reviewer understands the reason for the changes. This allows the reviewer to possibly question or suggest a better way to get the results. Otherwise, all the reviewer can do is suggest code standards based changes which can be done with a linter.”
“Adding the 'Right' people to the code review; i.e. the technical experts in that part of the code base. This is the most important thing! Putting enough people on the code review to ensure the minimum number of reviews happens on a timely basis. Adding a short blurb about what was fixed and why, documenting your best practices and being able to refer to that document in the code review”
“The code reviews I find most valuable are the ones that are timely (not after I've finished development of the story) and include some context on the nature of the change suggestions. Just saying "you should use X…" is not as beneficial as "by using X, it will provide Y". This allows everyone who sees the review comment to understand the reason for the suggestion, and an opportunity to learn more about best practices and improvements for using the development language being used.”
“A great code review answers two questions: (1) Does the code do what it is supposed to do? (2) Is the code easily understood and maintained? The first reminds reviewers they need to familiarize themselves with the feature or defect involved and in so doing they also become familiar with more of the overall software application. While the word "easily" in the second is quite subjective, it reminds reviewers to look at every level for unnecessary complexity and unanticipated side effects.”
“It depends on the context in which you are performing the review. A great code review for maintainability in JAVA might focus on code smells, method names and possibilities for refactoring. A great code review for security in C++ might focus on which libraries are being called how input is being processed and how pointers are being used. A good code review can be automated and performed with static analysis. But a great code review is dependent on people with expert knowledge and experience.”
A Standard Review Process
“Use predefined steps to start the process of code review because there should be a proper foundation on which the code review should start. Ex. Each developer should follow best practices and do a self-code review for himself/herself even before sending out for code review to other colleagues.”
“A code review should be a friendly guided digital discussion. The discussion needs to have parameters set by standards, targets, and a facilitator. With those guide rails in place, a code review is an effective way of sharing knowledge and increasing code quality. Without them, it has the potential of becoming a personal skill attack vector.”
“A great code review would be to ensure code follows the established patterns determined by the team, detailed notes by the reviewer back to the developer when there is an issue and ability to pin-point where the issue is in the code. Another nice element is to be able to acknowledge when you see great code and bookmark it to make it an example for others to follow. Lastly, a great code review tool should be able to inform the developer quickly and efficiently when comments are left on the review.”
“What makes for a great code review, includes but is not limited to the following:
- A pull request
- A well-formed and worded description
- Links for apps to test the changes
- UML diagrams explaining changes
- Testable code
- Readable code
- Maintainable code
- Code adherence to best practices
- Code allowance for architectural growth
- A Growth mindset amongst team members
- A positive attitude
- Great code review tools
- Badge integration for passing /failing unit tests and builds
- A REST API for integration
- Plugin integration
- A code review where team members have learned something good
- Good architecture discussions”
“It lists a series of tests to be carried out, as well as the important points in which different tests have to be carried out. Based on the results, the team presents the project to the end user so that, if approved, the tests will be shown and the user will realign the annotations of non-previews activities. The user is asked to assign 1 person for review, while the team performs tests in parallel in order to anticipate any possible non-pre-planned failure. The logs help us to maintain our analysis and preassign some assistance work, as well as selection of personnel that will intervene in the solution of the error or warning. This in order to anticipate the failures that the user will present in the near future and shorten the time of solution.”
- “Configure your linters.
- Don't get frustrated if someone complains about whitespace or formatting.
- Listen to why someone might recommend a different approach, even if your approach works. It might have a performance impact or compatibility risk... Just because it works doesn't mean you can't learn a better way from a colleague.
- DO REVIEWS DAILY so it doesn't snowball. Find a right time of day if you have a task that will spread a few days, stash it and do code reviews first thing in the morning.
- Check out the branch and see that it actually does what it should, don't just skim the code and assume automated testing handles the rest. Just because Jenkins said it passed doesn't mean it doesn't need review. Anything could be misconfigured or unit-tests not really testing anything. Check it out and review it, you might find problems in your testing configuration.
- Using proper tools like video conference help for occasional pair-reviews and team building.
- Always remember that as a reviewer, you are also learning. It's not all about finding what's wrong with the other guy's code. It's a good time to share thoughts and ask questions.
- Minimum 2 approvals before merge, depending on team size. Reviewers should include at least one expert and a few other people related to the work.
- Let someone else other than the reviewee merge it, it helps distribute guilt.”
“In my previous job, we had a senior dev who was most important during whole code review process. He had incredible domain knowledge and technical skills which allowed him to find even smallest possibility of bug. Also, he had a scanner in his eyes and could find even in biggest code reviews every space you missed. Also he was really eager to help and all that combined makes code review great: willingness to help, technical and domain knowledge, and scanner in eyes!”
“I find the most effective code reviews happen when developers are prepared to be open minded about their delivery approach. Both parties, coder and reviewer need to bring their experience and customer benefit to the table at the same time. Sometimes, just enough code is good enough - as developers unless your software is your product to sell - code is an enabler and the business value is being able to use it. Not how well it looks or follow pre-existing standards.”
“A good mix of senior and juniors reviewing code, leaving questions and observations, not just ticking the box. Ideally people from other teams not just the team you are on who have a less vested interest in just saying yes. We have also found a template explaining the PR and what it about are helpful to avoid unnecessary work by reviewer to understand what it is about. Also QA build generated on PR as sometimes it helps for a dev to just see what the built version looks like as well.”
- "Grooming a story as a team in advance - knowledge sharing and understanding expectations of advancing the story to 'Done'.
- Empowering all members of the Dev team (developers and QA) to write tasks, that identify specific buckets of work to complete the story, during Sprint Planning. This helps to share knowledge and provides additional insight into the "how" of the implementation.
- Sharing that a story is ready or nearing 'Review' status during the daily scrum”
Automation Analysis and Code Review Tools
"Automate away style nags. If there's something about style a linter-fixer can fix, add a rule. It's not useful for humans to argue about or use time to manually fix. Assess whether new tests are needed and try to help the reviewee learn something new without dictating. There mustn't be personalization of code (using adjectives such as stupid, lazy, bad). Ensure Boy Scouts' principle is allowed and participate in the editing, if you see significant changes are required.”
“First of all, a template of the PR summary, explaining what the developer should explain to reviewers. We focus on covering the WHAT with screenshots/gifs or a single sentence and then brief sentences explaining the HOW, focused on sharing the intention of the code to reviewers.
With these two, reviews are more relaxed as we make the life of the reviewer easier. Then comes the automation part: CI checks that validate that the PR increases the code coverage checks linting/if the code builds a productive artifact and even source analysis are great tools to detect things automatically, without the need of human intervention. Finally, PRs should be short, as atomic as possible, and focused on a single/a few requirements. Big PRs are harder to review, and this often means that the reviewer won't pay attention to some parts of the code.”
“Commenting on styling is a waste of two developers time. Use a tool to format and automate conformance. Code reviews are a chance to share knowledge and process, not gate-keep. Prefer cutting tickets and prioritizing new work to address issues that don't introduce functional issues for customers. Aggressively automate anything you find yourself doing repeatedly, including building tools that automate checks against best practices.”
- "Frame reviews as opportunities to improve code quality and for all participants to learn while not criticizing or casting blame.
- Use a tool to facilitate asynchronous code reviews, avoiding the need to gather multiple people to meet at once and capturing review comments and history.
- Integrating a code review tool into the team's workflow, so that reviews can be triggered by a pull request, commit, or other actions in various tools.”
Sufficient Time for Reviews
“Patience and coffee! A good code review should not be rushed. Every line of new or changed code has to be reviewed with the same level of concentration. Comments and suggestions need to be made (and received) without rancor.”
“The best code reviews are those that are provided by a colleague who has obviously taken the time to write out thoughtful comments, and make helpful suggestions. They take time.”
“Setting aside the time to go through the changed code properly, rather than just skimming over it. Consider what was meant to be changed, and whether it works as expected. Asking questions of the code author, rather than demanding changes. Having an understanding between members about how much back-and-forth should be allowed, and being clear that minor or cosmetic issues don't necessarily have to be fixed.”
“A great code review is when reviewers take the time to really dive into the review, and when all participants have the freedom to give honest feedback on the code and feedback of others. A great code review is one where all the participants have the chance to learn something.”
“You have to look at the code three times, each time while wearing a different hat. The first time, you're looking for adherence to standards, logical errors, variable naming, function return types - reading it line by line. The second time, you're actually looking at the existing code before and after this new code, making sure it's consistent and not orthogonal to existing logic flows. The developer shouldn't be patching code without understanding where they are. The final time you read the code, you're verifying that it solves the original problem and matches the requirements. It's far too easy to review the code itself and forget to validate the purpose behind it!”
- "Code review should be short. Big reviews tend to hang and get many raised concerns.
- Code review should cover only one specific feature/task. No mixed-in refactorings, no code-style fixes, etc. It makes review bigger and harder to follow.
- The author should always review his code before submitting it to others. That way all childish mistakes (such as typos, broken links/references, missed revisions in review) will be fixed and it will save time for reviewers.
- There should not be too many reviewers. One or two is enough. The more reviewers - the less they feel responsible for doing a qualitative review. If you want to invite many people in code review, then you probably should have done a design review beforehand.
- If there's a disagreement in a review, it should be dealt with in person: face-to-face discussion, a call, discussion in a chat. Otherwise, you'll be throwing arguments at one another via reviewing system at a very low pace (one comment per day or slower) and so the discussion can hang for weeks. Discussing the disputable concern in person is much faster.”
Collective Ownership and Team Engagement
“A great code review teaches a developer to care about his code. Following code style, code documentation guidelines and coding conventions helps readability. Improving readability leads to better code analysis. Better code analysis leads to easier bug detection. A great code review makes me learn something new or a teaches me better coding patterns. Code reviews improve consistency.”
“We started our code review practice to mostly improve code quality, assure design patterns were being properly followed and create a collaborative environment in a more natural way. Our results were almost instantly noted, as everyone accepted the habit and more experienced developers gained more space (without a hierarchy structure being imposed) to share best practices and code guidelines. Besides that, less experienced developers felt comfortable enough to share its own experiences and doubts, and the team grew more united. What I mean is, what makes for a great code review is great engagement and an important reason behind: code improvement and team engagement.”