The secret for effective code reviews
Code Reviews are first about people, and only then about code.
There is a lot of advice on how to review and less on how to be reviewed.
That's what I'll focus on in this article.
We'll go over:
The 4 main types of code reviewers and how to approach them.
Our role as team leaders in the process.
Anton here. This is the first article in 'Leading Developers', my new newsletter for team leaders, and aspiring developers. I plan to make the articles short (<5 minutes to read) and practical. I'm excited to start it, and I hope you'll enjoy the read :)
Let's start!
There are 4 main types of code reviewers:
The perfectionist - nitpicking comments, lots of arguing
The Guru - great comments, but tons of time to rewrite
The skimmer - almost no comments, quick approval
The Ignorer - can ignore your request for days
How to deal with each type?
The '𝗣𝗲𝗿𝗳𝗲𝗰𝘁𝗶𝗼𝗻𝗶𝘀𝘁' 🔍
You'll get comments about:
Not enough tests
Your variable names
Missing comments and explanations
Too many functions / missing functions
A request to refactor a code you haven't touched
Even after you fix everything, you'll get comments on your fixes, dragging the review for ages.
These people are the 'guardians' of the code base. They hate code that is not up to the standards, which they maintain.
At first, I thought they were just annoying. But when I really talked with one of them, and he told me why he behaved that way, I started to treat them with more respect.
Tips for success:
Set Limits. It's ok to disagree on programming styles (if it's not a company-wide standard).
When you feel you start arguing in the comments, set up a short call. Explain that you will address the important parts, and why you don't plan to address some of the comments. (Make sure to document that for future readers)
If there is a request for something you think is out of scope - open a ticket, explain the job, and make sure it'll be done at a later phase (don't use it as a tactic to ignore comments. Follow up).
The '𝗚𝘂𝗿𝘂' 🧠
The Guru will get into the deepest part of your code, find all the problems, and suggest improvements. But you might need to spend 3 days refactoring your whole code...
You are often afraid to request a review from them because you want to finish your task on time.
Tips for success:
Get them involved early, even during the design phase. Share your Work-in-progress (WIP) code. This will ensure the final review will not surprise you.
Those are the best reviewers for your learning. Aim to work with them as often as possible.
My first team lead is such a reviewer. When I review his code, I usually have almost nothing to comment on...
And I know that if I want a comprehensive review of critical code - that's who I'll talk to.
The "𝗦𝗸𝗶𝗺𝗺𝗲𝗿" 😎
It's often very tempting to request all our reviews from the skimmers - quick approval, no fixes.
This is of course a mediocre solution. It'll result in bad code, and worse - you’ll miss a great chance to actually learn something.
Tips for success:
Help the skimmer. Highlight in advance the areas you want a thorough review for. You can use comments like "Would love to hear feedback on this part".
Make sure the skimmer is not the only reviewer of your code.
Personally, I'm often a skimmer. I tend to approve too fast, without getting deep into the code (it's bad, I know). When people outline the most critical parts, it really helps me to review effectively.
The "𝗶𝗴𝗻𝗼𝗿𝗲𝗿" 😴
Ignorers don't do that on purpose. They might be the busiest developers, with tons of reviews assigned to them. Or they just don't have proper reminders set up, and are easily distracted.
Tips for success:
Be proactive - Let them know in advance your PR is coming.
Be assertive - GitHub and Slack might not be good enough. Set up a meeting: 'Anton / Ram - reviewing code', for 15/30 minutes.
If this happens often, talk to your manager. There should be some shared guidelines on the speed of response for Code reviews, otherwise, it drags everyone down.
Tips for team leaders
Having a good code review culture is essential for creating a strong team.
A part I loved from Google's guide for code reviews:
When code reviews are slow, several things happen:
The velocity of the team as a whole is decreased. Yes, the individual who doesn’t respond quickly to the review gets other work done. However, new features and bug fixes for the rest of the team are delayed by days, weeks, or months as each CL waits for review and re-review.
Developers start to protest the code review process. If a reviewer only responds every few days, but requests major changes to the CL each time, that can be frustrating and difficult for developers. Often, this is expressed as complaints about how “strict” the reviewer is being. If the reviewer requests the same substantial changes (changes that really do improve code health), but responds quickly every time the developer makes an update, the complaints tend to disappear. Most complaints about the code review process are actually resolved by making the process faster.
Code health can be impacted. When reviews are slow, there is increased pressure to allow developers to submit CLs that are not as good as they could be. Slow reviews also discourage code cleanups, refactorings, and further improvements.
But what can we do?
Don’t let a CR sit around because the author and the reviewer can’t come to an agreement. Help your developers set up those meetings. If needed - be there yourself the first times.
Have clear guidelines in your organization, on how fast CRs should be. Google suggests one business day as the maximum time (so first thing in the next morning, the latest). If it's a re-review, it should be even faster.
Help the developers decide who should review the code before starting the task.
A junior/new developer -> Should get gurus/perfectionists until he learns the company standards
An urgent fix -> get a skimmer as one of the reviewers. Make sure not to fall on an ignorer, it's avoidable frustration.
A critical part of your codebase -> Make sure at least one Guru will be involved in the review. Even better - in the design phase.
To sum up:
As a team leader, make sure there are clear guidelines for Code Reviews. Use Google's as a start: https://google.github.io/eng-practices/review/reviewer/
Understand who you want to review your code before you start the task.
Adjust your approach based on the type of person who reviews your code.