Code Reviews, Small Moments, Big Impacts

Multiplying

At HackerOne, we use the Spotify Engineering Framework, meaning we work in Squads. Each Squad is a mini-team focused on specific tasks, like a small startup within the company.

The essence of a Squad lies in their shared context and habits. When participating in a code review, several small decisions must be made that, when combined, have a significant impact on the software quality and the team’s identity.

It’s crucial to explain not just what is wrong but why. When only pointing out the issue, it may be fixed but without understanding the underlying problem, it will lead to the issue being repeated in the future. Investing time and effort in providing thorough explanations has a compounding effect. It helps the author gain new experience and can spark a discussion on best practices, leading to shared knowledge and team growth. This way MRs that you don’t review will still benefit from your feedback.

Providing examples of what good looks like or presenting alternative solutions and their trade-offs is equally important. Doing so helps to foster a culture of learning and improvement, similar to the principle of feedforward instead of feedback. Investing in sharing knowledge and creating standards can save time and effort in the long run, enabling the team to focus on more advanced tasks.

Taking time to flesh out your thoughts during code reviews builds trust and cohesion within the team, particularly in async environments where communication can be easily misinterpreted. It’s important to pay attention to the tone of your words and their potential impact on the author. Before leaving a comment, ask yourself what you hope to achieve and consider the possible effects. If the goal is to showcase your knowledge or inflate your ego, it will show through, ultimately transforming a bonding opportunity into a rift within the team. A bad example would be “Can you change this to that?” or “You didn’t write a test“ — this is very demanding. Instead, it could be, “What if we try to do this?” or “Have you thought about this?"

Lastly, sharing is a two-way street. Code reviews are not only about finding bugs but also a teaching moment. They help you keep your knowledge up-to-date but also ensure that you don’t miss out on any issues. Don’t hesitate to ask for more information about code that you don’t understand, even if just out of curiosity and make sure to read the documentation on the technology you’re working with. As you are also learning, it is good to show appreciation for the person. Sentences like “I really like this refactoring here! Thanks for changing this!” and “Wow, I didn’t know that we could use this in this way” can go a long way.

Executing — Optimize for Delivery

Growing as a team is great, but being a team with software in production is greater. The first goal of code review is to get a good quality solution deployed. Here are some ways to do that:

• Review within context: Ensure that you address the specific problem at hand and don’t allow for scope creep. Avoid making extensive refactoring changes to unrelated files or starting discussions on removing feature flags for a simple copy change.
• File a follow-up: If you do come across significant issues, note them somewhere, such as GitLab, so they can be understood and prioritized accordingly.
• Clearly communicate if your comment is blocking or not: Misunderstandings can occur, so it’s essential to communicate if a comment needs to be addressed before merging or if it’s a suggestion for future improvements.
• Choose your battles: Keep in mind that the solution doesn’t need to be perfect. Sometimes it’s more practical to focus on providing guardrails to prevent disasters than trying to achieve absolute perfection.
• Break MRs into multiple pieces: This approach makes it easier to review and prevents a small detail from holding up the entire process.
• Shift Left: Pair with your colleagues on important design decisions and over-communicate them over Slack. This approach helps to de-risk major changes and avoid last-minute surprises or issues.

Tips for Effective Code Review

• Review in Layers: Focus on the most important issues first, such as ensuring that the feature meets all use cases, before worrying about minor details like indentation.
• Review in Chunks: Break issues and merge requests into smaller pieces to prevent your attention from fading and maintain the quality of your review.
• Ask for Demos and Screenshots: You don’t have to manually test every solution yourself. Trust your peers, but request a demo or screenshots for high-stakes issues or potential edge cases.
• Don’t Be Afraid to Pause: If you’re hesitant to approve an MR, take a moment to review the purpose of the issue and the potential risks. It’s better to take your time and provide a thorough review than rush through it.
• Show That You’re Negotiable: Assume there is something you think should be really refactored. Stretching the discussion won’t help. You better talk like, “I know this feature needs to go live soon, so this change might be too much for this MR but is it possible to create a Gitlab ticket for this and pick it up in one of the upcoming sprints?”
• Use Examples: Try to find examples from the official documentation of that programming language. This is a really good habit and encourages people to do the same.
• Maintain a Checklist: Keep track of recurring issues in your squad and prioritize them in your reviews. Consider adding them to your MR template to help them become second nature.
• Prioritize Reviews: Merge Requests are valuable and should be reviewed promptly to maintain a fast feedback loop and avoid delays in the sprint. Too much open work can cause problems when combined, so prioritize your reviews accordingly.

Code reviews are crucial for ensuring high-quality software solutions and reinforcing the technical vision of your organization. Remember, you don’t have to implement all of these practices at once. Pick a couple that resonate with you and start there. And don’t forget to get input from your peers. What works for one team may not work for another, so it’s important to find what works best for you.