Code Review – Best Practices, Guidelines & Process Insights
Code review is one of the buzzwords everyone heard about. This technique strictly related to creating software is worth getting familiar with by everyone working in the IT environment.
This article was initially released on my employee company blog here
This article is dedicated to technical managers, CTOs, developers, and everyone who work directly with them. I will explain what code review is and why it should be applied to every development team process. Then I will point some pain points and tips and tricks – how to make a code review process more efficient, how to reduce the time overhead and mitigate possible conflicts between the engineers. You will also understand how code review makes codebase better and how to recognize the development team’s problems by carefully watching how code review is performed.
What is code review?
Before I can explain what code review is, I have to remind you about basic git (code version control system) concepts - branches and pull requests. If you are familiar with code review concept, you can skip to the next section.
When developers collaborate on creating new features and fixing bugs, they (hopefully) develop their changes on branches. In git, branches are separate “stages” where code is changed without affecting code on other branches (e.g., other developers features).
When a feature is completed, its author creates so-called Pull Request, which is a situation when some changes are requested to be merged to the main branch – so every developer creates features isolated, and in the end, everyone tries to merge them into the main codebase.
Finally, code review is the process performed during living pull request, where other developers check the code, comment changes, and perform discussions with the original author about proposed solutions.
Code Review Tools – What To Use?
Typically there is one platform developers use to keep and maintain their code repository. The most popular code review tools are Github, Gitlab, and BitBucket. You can compare them here. Most of them provide a broad set of features; code review is only one of them. Pro tip: You probably want to keep all code-related data in one place. If you mix tools, try to set integrations.
How To Do Code Review?
Code review is a discussion. It takes two to tango, so at least one developer has to be involved except the code author, but of course, more developers can – and sometimes even should – join.
Typically when code author opens Pull Request, he/she requests reviewers by selecting who should join the discussion – these developers should get notified. Of course, the review can be added without requesting – for example, a developer with free time can help others by adding extra reviews.
Code review platform allows developers to see a comparison between the original code and changes proposed by code author. Each line of code can be commented, as well as general comments can be added to Pull Request. Code review can end with three different outcomes:
- Accepted – when code is fine, and reviewer agrees to merge changes
- Rejected – where reviewer denies merging and requires changes to the proposed code.
- Comment – where a reviewer adds remarks but doesn’t make the decision about merging. It can be useful when PR is work-in-progress or developer doesn’t feel competent enough to vouch for checked code.
The code review process is a discussion, so sometimes requested changes are applied by the author, but sometimes code author doesn’t agree and discuss the problem with the reviewer. But this cuts both ways – sometimes it is a practical education process which ends with higher code standard, sometimes it’s a long and unproductive discussion (or even a flame!).
The goal of code review
There are several benefits of performing code review, but each team can focus on a different matter. Typically, the purpose you can look for will be improving general code quality and reducing the number of bugs by sharing knowledge of author and reviewers. Also, developers will educate each other on how to write better code and understand business problems.
How much time does the code review process take? Can I afford it?
Code reviews can take a lot of time indeed. This process should be added on top of a project’s time estimation. Just like the development itself, it’s tough (for every developer) to estimate how much time it will actually take. Time spent on Code Review will grow with the task complexity, just like the feature implementation. However, you can optimize this overhead, and I will give you some tips later in this article.
Keep in mind, that code review is just like agile development – it might feel like we add extra time, but it pays off later with better quality!
Why should you require code review in your projects?
There are many benefits of having an obligatory code review in your project. Let me show you some of them:
Early caught bugs
Most likely, the longer a bug exists in your code, the harder it will be detected and terminated. In time, more and more application’s parts can rely on defective code, making it more expensive to fix.
A code review will not catch all of them, but an extra pair of careful eyes can see what others don’t. Developers’ experience can vary a lot, one of them could have done a lot of work in previous projects related to, for example, dates manipulation, another one will be experienced in animations or performance. That’s a widespread situation when a developer already solved a problem once and just by looking at the code will know that this or that line will cause trouble in the future.
Improved overall code quality
It’s human nature that we try more when the effects of our work are evaluated by someone else. No one wants to be recognized as a sloppy nor untalented employee, especially developers! We, engineers, consider ourselves smart and most likely are open to prove it to others, don’t we? If developers can still talk about programming after work, they for sure will take time to share their opinions if they are asked to.
How does it improve code quality? Reviewers will review not only implementation (which, if corrected, also enhances the codebase), but how much code is readable (“I don’t understand the code, it should be simplified”) or the architecture (“I will need this piece of logic, but I can’t use it because it’s badly designed”). Reviewers can also mention that some code is duplication (“This is already implemented here, don’t write it again”) or check code against domain knowledge, which can be more extensive if the reviewer is working longer in the project.
Developers love to learn. They change jobs to try new technologies; they pay for workshops, spend weekends on reading, learning, and Hackathons. A well-educated developer is not only a benefit to your company but also – probably – satisfied one.
In my opinion, there are two ways to learn code the most efficient way – by coding and by reading the code. On top of that you learn on mistakes, but with the latter – on others’ mistakes.
So if a developer receives feedback on his/her code, he/she learns on reviewers experience and often – failures. Quick feedback is very effective, and the developer doesn’t have to see the code fails on production to determine that the code was damaged.
On the other side, there is a reviewer who is learning by reading. Every engineer has a unique experience, and the way problems are solved will be different. Sometimes even great programmers will have a very narrow point of view (“If you have a hammer, everything looks like a nail”), which can be problematic, but also widened by exposing to some other problem solutions.
Code review is a great learning tool for every programmer, but it can be especially beneficial for junior developers, who need to be assisted – and this is a great way to achieve it.
Of course code review will not replace learning budget and conferences, but will indirectly improve developers’ skills.
Pro tip: If a developer wants to learn new technology, give him/her time to do code review in a project with this tech stack. Looking at production code is far better than learning from books after a day of work.
Staying on the same page
In agile projects, there are daily scrum meetings (standups), and one of its profits is explaining to the team what was done by everyone. However, these reports focus mostly on high level, business problems.
Browsing Pull Requests can give developers similar insight about features developed, but reading the code is even a step further.
During Code Review, the reviewer should get familiar with business requirements (it’s already a huge thing to focus on others work) and later will learn what new modules were introduced or removed by others. Even a quick look at the code without deep dive into implementation details can be beneficial for the reviewer. If developer A is aware of new work by developer B, he/she can use it without duplication.
Onboarding new developers
Hiring new developers costs a lot – the recruitment process is only a beginning. Before a new developer will become productive and actually “earn” his/her salary, weeks or months will pass. The bigger and more complicated is the project, the longer will be time for a new developer to understand the codebase and business behind it.
Code review is a great solution to speed up developers onboarding. A new employee should read the current code, ask questions, and get familiar with how team solves problems and manages the project. Also, when a new engineer starts creating features, quick feedback by the rest of the team will reduce the time required to dive into the project.
Standardization of code rules and style
It’s better to have “some” style guide followed by everyone, then each developer having his/her own “perfect” style guide.
Code without standardized rules and code style is messy, hard to read, and problematic. Each developer’s settings will overwrite another’s, causing polluted code changes and inconsistency.
Code review is the discussion, so if any developer requires some change (eg, “Don’t use tabs, use four spaces”), the request should be discussed by the team. If a team agrees on one or another way how to do things, they just created a standard to follow.
The longer project lives, the more standards appear, and code becomes more readable – it feels like it was written by one guy, not ten. And more standard is also making onboarding of new developer easier!
As you can see, there are plenty of great reasons why development teams should introduce and follow Code Reviews in your company. But still there are questions – does this investment in time return? Should I still practice code review, even if it takes a lot of time, and my code probably will not become that bad if I resign?
We can measure ROI by the ratio between benefits in quality divided by time. I might be abstract, but one thing is obvious – the less time overhead code review requires, the better.
So let’s find out how to improve this ratio!
Code Review Best Practices – How To Ensure Maximum Efficiency?
Code reviews can be long, that is true, but just like any other process can be optimized. Here are some code review best practices that can be implemented in your company.
How to do code review like a pro? Let’s find out!
Two reviewers are better than one
It’s not easy to find a consensus if there are two different opinions on the table and two people who have to agree on one of them. Despite how strict computer science can be, there are a ton of problems which solutions based on very non-scientific premises and guesses. Both sides can argue and even fight to prove one’s right. Not many developers have in mind business problems (time!), so they can waste a lot of resources on pointless discussions.
Another problem could be the personal character of team members. Sometimes people get frustrated because they can’t push opinion against someone with more communication skills.
To solve this problem, it only takes to introduce a third developer (typically second reviewer), who will tip the balance and eventually agree only with one of the parties (or propose another solution).
If you have the resources, I recommend having two reviewers by default, but if you lack developers time, add extra reviewer if there is a conflict to solve.
Worth to mention that some problems better fit specific issues. Typical developers pair can be more effective when summoning expert from a particular domain to audit the code.
So – try to have two reviewers or at least one backup reviewer.
Make the Code Review a priority
Code review is a blocker for merging a feature, so if a developer requires some piece of code to be available for the new feature, it could be a problem. Required changes can be manually merged into the new feature before the start, but it can lead to code conflicts, on the other hand, the developer will wait to do nothing until changes are made.
As you know, blockers should be found as soon as possible and solved as quick as possible, so work is smooth as it gets.
So it is essential to make developers understand that pending code reviews (and overall pull requests) are actually more important than their own ongoing work (of course in a reasonable way, critical fixes are even more critical). It’s not an easy topic, because developers seem to care about their focus state a lot, but it can be communicated like this: make code review first thing developer do when:
- Starting work in the morning
- Going back from lunch
- Going back from Fussball/PlayStation/darts break
- Going back from meeting
So, if developers lose their focus anyway, make a habit first to check if there are no pending review by their peers.
Checking for new code review requests shouldn’t be too hard – it can be done in several ways. A developer can browse Github for open PRs with “Awaiting review from you” status. Also, each developer will probably get an email with a Code Review assignment. You can easily connect Github to Slack so that each open PR will be posted to the channel.
So – try to explain developers the importance of code review. Experiment with some rules to decrease code review waiting time.
Establish Code Review rules and write them down
Code Review process, like any other process, can use some rules, especially in bigger and more complex projects. More developers in the team mean diverse experience and practices. It is good but sometimes can slow down the effectiveness of work.
So, each team should establish some set of rules and write them down (don’t miss this point!). I don’t think there is much benefit of standardizing process in a whole company, but the team works together on a daily basis. There are two significant benefits of making this happen:
- New developer on the board requires education about existing rules. Don’t waste time on explaining every time the same things. It’s also frustrating to the new guy that he/she is corrected many times, instead of first reading how things are done in the team.
- In case of conflicts or misunderstandings, there is a single, living source of truth which can be referenced.
Here are some code review best practices that I always include in my work, which can help you improve the code review process.
- Only comment author can resolve comment – if code was corrected or after discussion author decides to fix it.
- Don’t mention the same problem many times. Don’t bloat the code, say it once and ask to fix everywhere.
- If there are pending, not resolved comments, the assignee is a code author who should fix or comment back.
- If there are discussions commented by the code author, the assignee is reviewer, who should continue a discussion or resolve comments and approve.
- Use labels to mark what actions should be next – e.g. “ready for review” “ready for the QA” etc.
The popular problem is when the code author thinks he/she is waiting for review, and the reviewer feels he/she is waiting for fixes/comments back.
So, establish some rules, write them down, and improve them over time.
Ensure that Pull Requests are good
An inseparable part of code review is a pull request. The review is done on changes someone request to “pull” to the main branch.
If the PR is good, a code review should be easy and fast. If PR is bad – code review will be exhausting, long, and “no one will have time to do it.”
The main rule of good Pull Request is to keep it short. Two hundred lines of code are easy to understand and to find problems. Two thousand lines are not. The cognitive load increase is exponential relative to the number of changes done in the request.
Short PR can be checked during a coffee break – long PR will cost literally many hours, and code review quality will decrease because the reviewer will be eventually too tired and confused to keep quality for a long time.
An extra pro tip is to make PRs well labeled. An author should ensure request is linked to a ticket system (Jira etc.), and there is reasonable description/readme for reviewers.
So, ensure developers spend some little extra time to create a good quality pull request, which will return in many hours of reviewing work.
Introduce Code Review audits between teams
In large organizations with many teams and projects, there are, for sure, a lot of approaches on how to manage development teamwork. This is a great approach to improve!
More experienced team (e.g., with senior engineers) can naturally develop good practices based on their experience, even without any management help. This is an excellent opportunity to extend their workflow to other teams.
On the other hand, you can observe a team with aggressive comments, long waiting time, and a broken process. The faster you know it and fix it, the better. Don’t trust that things will escalate soon enough.
Think about performing periodic audits of selected teams to observe the condition of the code review process (just like your audit management, scrum meetings, etc.!). Some teams will have problems, and some will be outstanding. Use this knowledge to share the experience of developers and fix possible issues on the early stage.
So, think about assigning CTO or other senior-level engineer or technical manager to perform audits of daily development processes.
Automate as much as possible
Developers prefer automating over manual and repetitive tasks. Code review might stand between code and human, but still, a lot can be delegated to the machine.
Set CI pipelines
First of all, PR should trigger a CI pipeline with tests and builds. This step runs a few minutes and catches a lot of problems that might be detected by the reviewer. Well written tests should cover a lot of quality problems, but without set CI, someone can merge it even if tests fail.
Connect your CI pipeline tool to Github and block possibility to merge until all checks pass. Don’t waste developers’ time to check code which will not even build.
Set code style formatters and linters
A lot of time wasted on code review is a ton of pointless comments related to tabs vs. spaces and how developers prefer code to be written (not what problems does it solve). Of course code consistency is important, but it shouldn’t bloat the code review process.
I recommend setting code formatter automated on Git pre-commit hook (which will run automatically) and linters on Git pre-push hook (which will cancel push it doesn’t pass). Also, linter should run on the CI pipeline. When these checks pass, there is much less to check by reviewer, who can focus on problem-solving.
Deploy branch to test environment
If you have a decent time assigned to DevOps work, I recommend you to set automatic deployment of open Pull Requests. Typically if a developer wants to check if the code work, he/she has to download and build it locally. It takes time, and sometimes it’s also a problem – if the QA department or product owner wants to check new feature before the merge.
Preparing automatic deployments of PRs will cost time to set up, but will let the team quickly check ongoing work, also by non-technical people.
Extra pro-tips from Ideamotive Team
Commit often and publish work-in-progress PRs
There is a bad practice when some developers wait for days until they show some work. Encourage them to commit often and push frequently. It will not only save the work in case of losing code locally (theft, broken device), but it will make the team more productive. Opening “Work-in-progress” branch will allow others to check the overall concept of changes over monitoring the implementation. This is especially important for junior developers when a reviewer can say after a few lines of code that this is just a wrong direction. It’s better to remove 50 lines of code than 500 or (5000).
Favor reviewers who know the problem
Probably there is not much sense in asking frontend developer to check backend code. If the team is big enough to choose from reviewers, it’s an excellent way to improve code quality. Some developers will better understand domain knowledge; some of them will specialize in some narrow tech problems. They will better fit review where they can give a lot of valuable feedback.
Modern git services make this easier by detecting and suggesting relevant reviewers based on historical changes or code owners.
Set live meeting in case of significant changes
If Pull Request is huge (shouldn’t be, but it happens), it can cost a lot of time to ping-pong online discussion with hundreds of comments. In this case, it could be better to set up a desk meeting where the code author explains other changes.
How to reduce conflicts during code review?
Code review, just like any discussion, could cause conflicts between developers. Also, it’s based on giving feedback, which is very hard for both reviewer (to do it with empathy) and author (to accept criticism).
Of course, the possibility of conflicts depends a lot on the individual character of developers, but if it happens, there are few rules, how to reduce it in the future.
Write down as much as possible
Conflicts can emerge when there are two different decisions, and parties don’t want to give up the idea.
However, a lot of decisions have been made already – but they are lost in time. I mentioned code style guide before – this is an excellent example because no one will argue about “tabs vs. spaces” if this has been officially decided and written in the project documentation.
There are different ways how to create documentation like this. I like tools like Notion to develop and maintain wiki-like pages with e.g., decisions made by business or rules established by developers.
Doc like this can be referenced during reviews to avoid unnecessary conflicts.
Educate how to do a good Code Review
There are some rules about how code review should be performed to achieve its goals. Not every developer is aware of that.
The review should be based on facts, not guesses. It’s not a place to think about any personal matters – the only thing that matters is code change and how it affects the project.
Each comment with feedback should include the explanation (“this is wrong because”) and proposal of a better solution (“maybe try to do it like this: …”). Extra points for linking to external sources like documentation so that code author can learn for the future.
And the most important – developers have to understand that they have a common goal which is the best code they can do. This is not a place of personal ego or competition who is a better engineer.
Introduce extra developer to solve conflicts
In case of ongoing conflict where there are two developers can’t agree with, you can invite the third developer to tip the scale and solve the problem. This extra engineer is not only an additional opinion but also a mediator between conflicted two.
How does the code become better thanks to code review?
There are a lot of benefits I mentioned why Code Review is an effective way to improve overall team performance. How does it affect existing codebase?
Code review is a discussion place, which handled properly should end with some actions. One of these actions is standardized code ruleset, which can be checked later on the whole application.
With Code Review developers will be updated with changes done on the project, so they will not be surprised “where it came from.” They will know that some functionalities were added and they can reuse them later, without duplications. Also, they will understand about deprecations and refactors, so they will try to follow.
During Code Review, many minds join forces to create the best solutions. One developer will not be great in every engineering area, but with feedback from others, code can be far better than initially.
How to tell something is wrong based on code reviews?
I always encourage to analyze, monitor, and draw conclusions from every piece of data we have, including processes.
Pull Requests with Code Reviews produce the information we can follow to improve in the future and check the condition of the development team.
I recommend checking at least a few factors, which are:
- How long PR is open
- How much time is between opening PR and the start of Code Review
- How long Code Review takes
- How much time is between accepted PR and merge Pull Request
- What is the ratio between Code Review time and Pull Request length
Of course, the perfect scenario is when code review is done quite soon after opening PR (few hours), time last during open CR should be no more than a few days (this takes not only discussions but also fixing issues, testing, etc.) and merge should be immediate after CR is accepted.
By watching this in time, you can see how different factors might change relative to other factors. Maybe CRs started to take a long time because the new developer appeared and gives a lot of feedback? Then perhaps it’s worth to check if the feedback is valuable (which means code was bad before), or maybe it’s just nitpicking?
If the time between PR and CR increases, maybe there are too many features, so developers can’t check other code? Maybe PRs started to be too big, so they don’t want to do it?
Also, long time between CR and merge can mean a broken deployment process.
In general, the more data you get, the more you conclusions you can make and faster you can improve. I recommend to monitor as many processes as possible, CR/PR is only a small example.
And… That’s it! I hope you can use this knowledge to implement perfect code review in your company or improve existing ones. To be honest, there are voices against reviews, but every company is different, and there is no other way than an experiment, draw conclusions, improve, and experiment again.
Send this article to both managers and developers to maximize the effect!