#12 - How to do great Code Reviews | Roojuta Lalani, Staff Software Engineer at Slack
When you review code, don't Slack off.
Hey there, another week, another world-class leader! Roojuta Lalani, Staff SWE at Slack shares her secrets to great code reviews in the following article.
If you’re not subscribed yet, get an actionable guide on a specific engineering leadership topic each week in your inbox, just by clicking on a button:
Let’s go!
Roojuta Lalani, powering Slack’s automation workflows
Code reviews are a critical part of an engineer’s work, both as the person giving and receiving the review. In over a decade as a software engineer, Roojuta Lalani, Staff SWE at Slack, had the opportunity to work on tough technology problems that solve real-life problems, at companies like Salesforce and Slack. Code reviews have been an integral part of that process.
Her career began as a junior engineer on a team that was doing some really important work from the customer’s point of view, although quite simple from an engineering perspective. She found out about a new team that was being formed for a brand-new project and joined that team. Being a founding member of any team gives you time to understand the domain, customer, and technical decisions. In that position, she worked hard to go above and beyond and got recognized as a star performer. This earned her her first promotion as a lead engineer on a team.
The next project she worked on was a combination of networking and building credibility with engineering leaders. She joined a startup that Salesforce had acquired, which exposed her to a new tech stack, different engineering practices, and small company culture. She realized there that she loved the agility and lean processes, which led her to work on multiple acquisitions within Salesforce.
Eventually, she felt ready for a big change and joined Slack, where she is a Staff Back-End Engineer focusing on Workflow and automation. When she joined, Slack’s platform/automation team was working on a major release, and that sounded like a place where she could make a big impact.
At Slack, Roojuta has been focusing on building a no-code automation tool and getting it ready for general availability (GA). This involves ensuring they launch a high-quality product while tackling some forward-looking architectural challenges. One of her toughest projects involved deprecating an older product version, which reduced engineering maintenance and seamlessly migrated a large amount of data to the newer version. All of this, while ensuring that all automations continued to function smoothly. This transition was critical as many customers relied on the automated system for daily operations, requiring flawless execution, high availability, and a low tolerance for errors.
Besides Slack, Roojuta is a mentor and speaker for Plato and TechWomen, as well as an advisor for Builders Fund.
You can follow Roojuta on LinkedIn.
How we do Code Reviews at Slack
A brief introduction to my guiding principles
Before we get into my specific advice for code reviews, I’d like to share my guiding principles that shape how I approach all my work.
“Our work as engineers cannot occur in silos. We need to work together to support and learn from each other, to lift each other with best practices, and align around the same vision”
The points outlined on the slide above are the mental model that helps me determine how I show up at work every day. These principles help me ground myself when things get difficult in a personal or professional setting.
Essentially, I believe that our work as engineers cannot occur in silos. We need to work together to support and learn from each other, to lift each other with best practices, and to align around the same vision. And code reviews are a big part of this—when done well, they enable others to do their job.
Let’s take a closer look at what a code review involves.
What is a code review?
Like many people these days, I was curious to see what ChatGPT had to say on this subject, so I asked ChatGPT to define the concept of code reviews. It produced the following:
A code review is a systematic and collaborative process used in software development to assess and improve the quality of code written by developers before it is integrated into the main codebase or deployed into a production environment.
I think there are a few aspects of this definition that are worth highlighting:
Systematic: This is a system for which you and your team determine how to implement the code review process. It includes general guidelines that are widely understood and agreed upon.
Collaborative: The goal is to strive for collaboration and learn from each other.
Assess: This process involves some assessment to gatekeep what goes into production.
That’s not bad, but let’s consider another option. GitLab defines a code review as “a peer review of code that helps developers ensure or improve the code quality before they merge and ship it.”
I agree that most companies have some sort of process where their code is reviewed and tested before being merged into production. It often goes through multiple rounds of testing after being reviewed to make sure the code works as expected, is of high quality, and does not break any existing feature (known as regression).
“There’s no single way of doing code reviews—they can vary depending on the company and the project in question, but ultimately most engineering teams perform code reviews to ensure quality and consistency.”
That being said, I’d add that a code review is not always performed by a peer. Sometimes domain experts are asked for their opinion before merging a critical code block that is either high risk or has a high impact. Sometimes, code has to go through a select set of people who are code owners of that block/package of code. This usually happens when the code itself is sensitive to human errors.
However, more often than not, most code reviews are still done by peers who have enough context of what is happening in a certain project and provide an opportunity to get another pair of eyes on it. This is a great way to catch bugs, get on the same page, share knowledge, and make sure patterns stay consistent across the code base.
In other words, there’s no single way of doing code reviews—they can vary depending on the company and the project in question, but ultimately most engineering teams perform code reviews to ensure quality and consistency.
How do code reviews work at Slack?
As I’ve outlined in the previous section, there’s no single way of conducting code reviews. But you’re probably curious to look at one specific example, so let me share a bit about how this process works at Slack.
There are many teams and repositories where code is merged, but when it comes to the monolith where a lot of code lives, code reviews are pretty streamlined.
Before we merge any changes into the main branch, we need at least one person to review the changes. For the most part, there are no strict rules built into the system about who that reviewer should be, but it implicitly trusts that we know who could potentially review this code path. Apart from code reviews, expectations are that we add/update some sort of tests to the pull request (PR) and make the linter happy.
When it comes to the timeline, the team that’s working on a particular project is usually on top of it and considers it to be the highest priority. Because you cannot merge without approval, this is very important. But as you can imagine, a person can get too many emails and forget about or overlook some reviews. For this purpose, my team uses Slack automation.
In our team, every time someone creates a PR, they can run that workflow (automation which anyone using Slack can build to enhance their code reviews) which includes a brief description of a PR, a link to Github, and the reviewers. This posts a message in the team channel and also mentions the reviewer, which sends them a notification. We also use reactions and emojis to let others know that they are looking at it or it has been reviewed.
3 factors that determine the quality of a code review
Now that you have a better idea of why code reviews exist and how you might set them up on your team, I’d like to share how you can create an effective code review. Three factors determine the quality of a code review: the pull request (PR), reviewer, and reviewee. We’ll look at each one in more detail.
Factor 1: The Pull Request (PR)
A good PR meets all these criteria:
It’s crisp and clear. You know what it includes. The description is key. It helps you be more productive as a reviewer and avoid a lot of back and forth.
It should be easily understandable. If I have to go back and read three different textbooks to review someone’s code, either I’m not being productive at my job or I’m not the right reviewer.
Small and succinct. As a great engineering leader, you need to know how to break up a larger task into smaller chunks. At the same time, the code review needs to be shippable to production.
Additional context is supplemented by comments/specs/links to Slack conversation. Usually code reviews shouldn’t require other documents, but there may be times when you need to set up a meeting, especially when it’s being reviewed by an architect or domain expert. In these cases, you can spend 20–30 minutes going over what you’re trying to achieve, then give the reviewer time to review it asynchronously. This will be more productive than just sending out the PR. If that person is a domain expert and doing a lot of PRs, they don’t have a lot of time, so providing extra context can really help them out. If you’re the reviewer, how do you know when to request a meeting? I’d recommend doing this in cases where the main problem is not being solved, the motivation is not clear, or the PR is deviating a lot from what you expected it to be. Keep in mind that sometimes having an actual conversation or a huddle can be more productive to realign and make sure everyone is on the same page, so don’t be afraid to request a quick meeting.
Most companies have a PR template you have to fill out before you submit a review. At a basic level, we want to know what we’re trying to do: Is this a PR because of a bug, because we’re trying to improve something, or is it a work item?
Here at Slack, our PR template contains the following elements:
Summary
Reason for this PR
Link to bug/work item: We always try to provide links to the bug or work item so you can see all the PRs that are associated with the work item. It helps when you’re going back and looking something up to know why you did certain things.
Before and after pictures: For a lot of UI features, we have before and after pictures, which helps to visualize the feature.
Monitoring
Where did you test? Manually? Added unit test? Automation?
In case of an incident, can you safely revert the PR? Not all PRs can be safely reverted in the case of an incident. What about this one?
At Slack, this checklist is added to the PR template, so anyone creating a new PR has the option to go through it and check for themselves if they have followed all the steps. However, at Slack, we believe that engineers will do the right thing and nothing is enforced. We consider this as a gentle nudge to all the PR creators. It’s a good reminder of everything you should do, but once you’ve done it for six months to a year, you probably don’t need to look at it anymore because it will become second nature to you.
If you don’t currently have a template or checklist in place, I’d recommend going back to your teams and creating your templates and checklists. Every team is different, so what works for us may not work for you, and involving your engineers in the process will likely make them more motivated to use the tools.
Factor 2: The reviewer
The second factor in a quality code review is the reviewer. I’ve outlined the qualities that I believe are essential to being a good reviewer below.
As a general rule, I’d encourage you to be nice when giving feedback. The way you deliver your message can impact whether it’s valuable or whether it discourages the other person from coming back to you with questions in the future.
When it comes to the topic of preference vs. problem, the Silicon Valley fans out there might remember the episode where Richard breaks up with his girlfriend because one prefers tabs and the other prefers spaces in their code. This is a perfect example of letting preferences take over. Remember to ask yourself: Am I focusing on the real problem? Am I catching bugs? Am I finding ways to avoid future tech debt? This is where you should be focusing your attention.
And remember that code reviews are for early detection. Don’t blame or shame anyone for asking questions, for “a bad PR” or for doing something that doesn’t make sense to you.
Everyone comes from a different perspective and experience, so try to be curious and empathetic. They may be missing some context that you have. Give them the benefit of the doubt.
Here are a few additional tips depending on who you are as a reviewer:
If you’re the reviewee’s manager: You’re probably not usually involved in the PR reviews, but you can track the state of a particular PR or the general performance of an engineer.
If you’re a domain expert: Focus on high-level vision, frameworks, and extensibility. Don’t waste time on the nitty-gritty details.
If you’re a junior/new engineer: Can you understand the code? Can you understand the problem we are trying to solve? If you look at this code, could you think of any specific scenarios where this code would break? Is there an optimal way of solving a certain problem?
Factor 3: The reviewee
And of course, the third factor in a good code review is the reviewee. Here are a few of my guidelines for reviewees.
As the reviewee, be open to feedback. This will help make the code better and make you better as an engineer, too.
Finding the right reviewers means you don’t just have your friend do it so you can merge it before the end of the week. When you take this approach, you’re missing out on the point of the review. Ideally, you’ll find someone who comments on your text and says all the negative things about your design.
Own the code you write. We have accountability for the code we check-in.
I generally feel like a good engineer is someone who will leave the code better than they found it. It’s super valuable to leaders when you find the problem in the code and fix it yourself. For example, when you are merging a bug fix—which is maybe one line of code—or when you are deep in the code, as a good engineer, fix your bug, see what’s going on, and report on stuff. If I see a function that is too long, I don’t have to fix it in the PR, but I create a ticket to make it smaller, and more modular so the code becomes more maintainable. If you see there are no tests, that’s not an excuse for you to check in without tests. It’s a good idea for you to bring it up in your next scrum meeting and say maybe we need to add more tests to this class because we don’t have any. If you see something that’s broken, tell others or fix it yourself. Own it.
Again, being empathetic is key. Try to get to the bottom of why someone is giving you feedback rather than just saying, “They don’t know what I know.” These conversations are healthy, so try to engage in an open-minded way.
Code review and its impact on culture
Going back to the elements I highlighted at the beginning of this article, making code reviews a regular part of your process can have a positive impact on your team and company culture in several ways.
Systematic: Often systems, rules, and processes are defined by people who are not the ones following them. But if you make this a team agreement (letting members of your team help design systems and templates that work for them), you will increase motivation and participation.
Collaboration: Code reviews provide a collaborative opportunity for learning from your colleagues. When you take a “no questions are dumb questions” approach, you create a psychologically safe space where engineers feel like they belong.
Assess: Remember that code reviews are meant for gatekeeping. They’re designed to evaluate the code’s quality rather than the author responsible for it, ultimately making the code better suited for your company’s overall vision.
Improve: The purpose of code reviews is to improve your code’s quality and ease of maintenance.
People will often ask you what the culture is like at your company. And the answer is not straightforward. It depends on the team. There are microcultures within big enterprises. You are the culture. You are the one who’s making it a good place to work or not. If something is broken, you can fix it—and code review is a great way to take a peek into a team’s culture.
“If something is broken, you can fix it—and code review is a great way to take a peek into a team’s culture.”
In my experience, code reviews function as the gatekeeper of bad code and prevent it from reaching production. This has a significant impact on the culture, especially in a big company. Using code reviews as a tool to coach, mentor, and build your team members up is a good way to keep the culture “green.”
Creating an environment of trust and empowerment within a team is crucial for fostering creativity, productivity, and overall satisfaction. By allowing engineers to have control over the decision-making process and empowering them to make choices regarding the code they write, you not only promote ownership and accountability but also encourage innovation and continuous improvement.
Prescriptive code reviews, while potentially helpful in maintaining standards and identifying issues, can sometimes stifle creativity and discourage individual initiative. By providing a checklist as a guideline rather than strict rules, you give engineers the freedom to apply their expertise and judgment to the specific context of the project.
At Slack, this approach has likely contributed to a culture where engineers feel valued and respected, leading to higher levels of engagement and job satisfaction. Trusting team members to make informed decisions fosters a sense of ownership and investment in the success of the project, ultimately driving better outcomes.
“Fostering an environment where questions are encouraged and valued is vital for the growth and development of new engineers, as well as for the overall effectiveness of a team.”
When I joined Salesforce I saw a poster outside a meeting room saying, “No question is a stupid question.” As a junior engineer, that truly made me feel like I could ask more questions during my onboarding and throughout my tenure there.
Fostering an environment where questions are encouraged and valued is vital for the growth and development of new engineers, as well as for the overall effectiveness of a team. Negative comments or attitudes towards questions can create a culture of fear or hesitation, where individuals may be reluctant to seek clarification or guidance, ultimately hindering their learning and progress.
Embracing questions as opportunities for learning and growth can lead to greater innovation, problem-solving, and overall team success. It's essential for organizations to actively cultivate this mindset through both explicit messaging, such as the poster I encountered, and through the behaviors and attitudes of team leaders and colleagues. By doing so, they can create an environment where curiosity is celebrated, knowledge is freely shared, and everyone has the opportunity to thrive.
You now have some templates and best practices to help you make code reviews a regular part of your work. I’d encourage you to consider how you can bring these concepts to your team and what kind of culture you’d like to create as a result.
Looking forward: Considering the impact of AI on code reviews
There’s one big question I haven’t covered yet, which is about the role of AI in code reviews. Will AI replace the need to do code reviews at all?
There are a few ways AI can already be used within the code review process and I’ve seen many AI tools in place in some form at Slack. These include:
Code linters and static analyzers: Tools like ESLint (for JavaScript), Pylint (for Python), and RuboCop (for Ruby) can automatically check code for style violations, potential bugs, and adherence to coding standards, like notifying you when you check in something that there’s an extra space at end of the line.
Code review bots: These are AI-driven bots that can be integrated into your code review process to automatically perform tasks like checking for code style violations, identifying security vulnerabilities, and providing feedback.
Natural language processing (NLP) tools: Some AI tools use NLP to analyze code comments, commit messages, and documentation, helping to identify issues and improve code comprehensibility.
Test detectors: We have one and it complains every time you check in something or put up a PR without a test in it, which is a bad practice. You don’t want to do that.
PR length checker: This tool tells you how long your PR is and if it’s generally considered good.
PR description checker: A lot of times we see people just putting up a PR and saying “Change the variable type.” Small descriptions are generally not productive and not a good sign of a PR.
Code health checker: Every time we check in code in Slack, there’s a little thing that complains, saying you decreased the health score by one because you didn’t add a test or follow one of the other rules. Everyone should strive to keep that health score as high as possible.
Based on my experience, I see that AI can be used to find bugs efficiently. These tools may not replace code reviews but are likely to become more widely accepted as a part of code reviews in some way or form.
As I’ve shared throughout this article, I believe code reviews provide a good opportunity for performance evaluation, team building, and mentorship. If AI ends up replacing human code reviews, we will need to look for alternative ways to help team members feel connected and learn from each other’s experiences.
And that’s a wrap! Thanks to Roojuta for her insights. If you liked the article, click the big button below to share it:
See you next week!