On September 20th, we did a webinar called Becoming a Code Review Master. Guest speaker Curtis Einsmann, former AWS engineer and creator of the Master the Code Review course, joined our Codacy CEO Jaime Jorge in an engaging talk about code reviews. During this talk, they discussed:
- How teams can create a better code review process;
- What are some common mistakes developers make while code reviewing;
- Tips for creating a good Pull Request;
- How can we keep learning to level up as a developer;
- Plus, a live Q&A to answer your questions!
In case you missed the talk live, don’t worry: you can (re)watch the recording here or below 👇
A live talk on mastering code reviews
During the live talk, Jaime Jorge, Codacy CEO, moderated the conversation and asked interesting questions to our guest speaker. You can check the detailed answers on our video recording of the live talk — we even give you specific time frames! But we’ve summarized the key points for you to read 🤓
What motivated you to start creating your content about code reviews? (00:03:49)
When Curtis started his career as a software engineer, he struggled with code reviews. He would get a lot of feedback on his code, sometimes with 50 or more comments. He would address them and then get another 30 comments. The feedback was kind and helpful, but it was a struggle. Plus, when other people sent their code to Curtis to review, he was unsure what to look for. He didn’t know how to give good actionable feedback.
He ended up learning through trial and error and getting more experience. Eventually, Curtis could ship his code reviews faster and help his peers level up through actionable feedback.
This is a struggle many developers face, but they are not talking about it enough. The lack of conversations about code reviews and their challenges motivated Curtis to go all in on this specific but important niche.
Many teams and developers face the same problems. If you were to give some tips to them, what would you say? (00:06:40)
A good code review process enables your team to ship functional quality code and ship it with speed so that customers can get their hands on it and start using it.
- Give it the importance it deserves. Code reviews are vital for the quality of your code and, ultimately, your product.
- Automate as much as possible. Static code analysis tools, like Codacy Quality, can scan your code for coding style, language best practices, code coverage, and security vulnerabilities.
- Make sure to set expectations. On the author’s side, explain what should be in a Pull Request and what the description should look like. It’s also a good idea to have PR templates. For reviewers, explain what and how they should comment.
- Document your principles. Drive down all code reviewers’ disagreements regarding principles and coding best practices by documenting them and explaining the why behind each decision.
How do you balance the formalisms and expectations with speed? (00:13:07)
You need to identify the points that the reviewers should be focused on. The reviewer should approach the review understanding the trade-off between speed and quality and if the author is optimizing for one or the other.
For example, if the author is optimizing for speed due to tight deadlines, they should also mention this in the Pull Request description. You should also set these expectations at the team and product levels.
Which are the most common mistakes people make when reviewing code? (00:15:00)
A big mistake is being ambiguous or not giving the reason behind a piece of feedback. If you have a compelling reason, the author should be able to understand it, implement it, and also not make the same mistake in the future. If you don’t explain why it opens the door to this back-and-forth discussion and debate. Healthy debate is encouraged, but it isn’t necessary for every piece of feedback.
A more objective mistake, especially for junior engineers, is looking at the code change itself and trying to find flaws within the diff. They should also consider the code change within the context of the entire system, but that takes some time and experience.
Another mistake is being unkind. Giving commands and orders is a long-term losing game with deteriorating relationships. In code reviews, soft and social skills come into play, and many relationships can be formed here.
What are the major differences between code review in a company and the open source world? (00:18:50)
Asynchronous code reviews originated from open source. Pull Requests were created as a gatekeeping mechanism to prevent people you don’t know from adding to your codebase without you reviewing it first. And that practice transitioned into companies.
That migration shift kind of changed the mindset. In open source, a person you might not know is coming and adding code to your system, and it might also be true that you are not working with them for a long time. So maybe you don’t need to focus on being kind as much; you don’t focus on building that relationship.
But in a company, you are working with someone long-term and on the same team; ideally, it should be someone you trust. So it’s a different relationship dynamic. When you comment on the code review, you comment on the code itself; you don’t comment on the person. But you need to remember that there is a person behind the code. Empathy and building a relationship come in, mainly because you will be working with this person daily.
What do you think is the role of a manager in the code review process? (00:22:19)
Depending on the organization, the manager plays a different role, but they should be very involved in developing and documenting code review guidelines.
Several times managers are not into the code and the code review process, but it’s good practice if they scan the code reviews now and then just to see what the team’s culture is like. They should also talk with their engineers and ask how the code review process is going.
Can you offer any rules of thumb for a good Pull Request? (00:23:46)
A good Pull Request should be small. Large PRs are very difficult to review and carry more risk when shipping those changes to production. But, to create a small PR, your task has to be small as well, and sometimes developers might be disengaged with the project management process, and how tasks are broken down, estimated, and scoped.
Then there is actual code: it needs to cover edge cases, have the right level of abstraction, be logical, readable, and tested. This comes from improving your craft over time.
There’s also opening the Pull Request itself. Again, depending on the organization, you might want different levels of detail in the description. Still, you should explain the problem and why your solution is the best way to solve it.
Finally, you should review your code. Every time you open a PR, you can review it on an internet browser, not on the IDE, since that change of environment can put you on review mode and makes you analyze the code with a little bit more skepticism.
How much time should a developer or development team spend reviewing code per week? (00:26:54)
It depends. It takes much longer on legacy systems because you have to account for all these different edge cases, components, and side effects. But it always takes longer than you think.
The process of a code review is that you first need to understand the code, the code change, and the parts it impacts. And then, you have to find flaws and give good feedback. That’s a challenging thing to do, and it takes time.
What tips can you give developers to continue learning and evolving? (00:30:05)
Performing code reviews for others is a great way to level up your skills and learn. Watching how other developers have approached a problem exposes you to different problem-solving strategies that you can adapt and leverage later.
Social media is what you make of it. You can choose who you follow, and when you are scrolling, you want to make sure that you are putting good information into your brain. That’s a good way to stay up to date as well. Tech Twitter and Hacker News are some examples.
You can also read deep technical books. Some good examples are Clean Code, A Philosophy of Software Design, Refactoring, and Domain-Driven Design.
Finally, you should improve your soft skills. Code reviews are an example of a process where soft and social skills are fully displayed because it’s all about collaborating with other human beings.
Q&A time
After Curtis and Jaime’s talk, we opened the floor to all the questions the audience might have. We were delighted to receive a tone of questions! We’ve listed them for you:
- Do you have an opinion about this statement “one mob/pair is always better than an async review”?
- Do you think code reviews can be automated someday?
- If you do not have automated code quality tools configured in your pipeline, is it valid to send a notification in a PR to make it compliant with the tool you are not using?
- Should a reviewer always make sure they check out the code, run it locally and test the functionality?
- Sending short PRs is good for the one doing the review. But, thinking about the team workflow, it could also lead to some blocking issues. How could we avoid team blocking caused by incomplete features of small PRs?
- Is there any recommendation for approvals, e.g., two approvals, different teams, or even a security champion?
- Sometimes we are eager to review every line of a PR and end up asking a lot of questions or requests for change. What would be your advice on this? How much can someone review until it becomes too much?
- Do you think it is important for the author of the PR to respond to all reviewer comments?
- Any tips on how to respond to someone who has given you a particularly unhelpful, harsh code review?
- Any tips for managing time between working on your tickets and reviewing others’ PRs?
- What’s your opinion on blind code reviews?
- If a feature was written in a pair coding session, do you feel it is still necessary to subject the code to a normal amount of scrutiny in a PR?
- What do you think about code suggestions when reviewing? How often do you think it’s good to use them?
- Do you use any automated code review system prior to actual standard code review?
Thank you to everyone who joined live and those who watched the recording later on! Let’s keep the conversation going in our community.