Chapter 1: Introduction to Code Review Standards

During my tenure at a major tech firm, we maintained extensive documentation on best practices for code reviews and coding style guidelines. This valuable information has been made publicly accessible, covering topics like code review practices and Java style guidelines. These standards encapsulate best practices across various programming languages and are regularly updated to align with new language features. Given that these guidelines are crafted by some of the brightest minds in the industry, it’s important to note that they serve primarily as recommendations, which can sometimes be subjective. For instance, the debate over using tabs versus spaces remains ongoing (let’s face it, tabs are clearly the superior choice).

Despite their seemingly stringent nature, these standards significantly contribute to the consistency, structure, and thoroughness of code produced at large tech companies. Most importantly, they provide crucial guidance to junior developers who are still learning the ropes of writing clean and understandable code.

Transitioning from Google to Lyft, I identified two key issues within my team's code review process:

1. Pull request iterations often exceeded a day before receiving feedback.
2. The quality of code reviews was lacking—either overly focused on minor details or simply rubber-stamped without proper examination.

These issues had a detrimental effect on both our team's productivity and the overall quality of our code. To address these challenges, I developed a set of code review standards and guidelines tailored for our team. These guidelines were created by incorporating suggestions from my colleagues, leveraging my own experiences as a software engineer, and utilizing various online resources. Feel free to adapt these guidelines for your own teams or projects:

On Approvals

• Approve a pull request (PR) if the modifications are safe to deploy as they are.
• Lean toward granting approval when all comments are minor suggestions. Withholding approval suggests that the author must revise their code before receiving the green light.
• If the changes are not safe to ship as-is (e.g., they might lead to outages), lock the PR to prevent accidental merges.

On Review Timeliness

• Set a service level objective (SLO) for the duration between a PR request and its review. Getting the team’s consensus is crucial for documenting and aligning expectations. At Lyft, we established a 3-hour SLO. For example, if a reviewer was tagged at 1 PM, they were expected to complete the review by 4 PM. If tagged at 5 PM, the review was to be done by noon the following business day.
• If a reviewer cannot meet the SLO, they should comment on the PR to set expectations with the author.
• If the SLO is not met, the author should feel empowered to follow up with the reviewer. Note that the SLO serves as a guiding principle rather than a strict enforcement mechanism.

On Authoring a PR

Do:

• Assume the competence and good intentions of your reviewers.
• Provide ample context for the changes, including a description, links to relevant JIRA tickets, or technical specifications. Completing the PR template is the author’s responsibility to aid the reviewer in understanding the PR’s objectives.
• Review your own code prior to inviting feedback.
• Keep your changes concise. Smaller PRs are quicker to review and increase the likelihood of catching bugs. Aim for a PR that is around 250 lines, including tests. Research shows that processing large amounts of information becomes difficult beyond 400 lines of code.
• If you disagree with feedback, explain your reasoning and provide supporting materials (e.g., coding style guides).
• Retag your reviewers if the review SLO is breached.
• Involve the approving reviewer if significant changes are made after approval.

Don't:

• Rely solely on your reviewer to identify bugs; you are responsible for your code’s quality and accuracy.
• Include irrelevant changes that do not pertain to the PR’s goals. While it might be tempting to refactor unrelated classes, this can confuse reviewers and lead to unnecessary context switching.

On Reviewing a PR

Do:

• Assume competence and good intentions.
• Request clarification if you lack sufficient context for the code being reviewed.
• Encourage the author to split the PR into smaller sections if it exceeds 250 lines.
• Focus on the overall correctness and substance of the PR rather than nitpicking. Clearly label suggestions as "nit" to convey that they are minor.
• Proofread documentation strings.
• Positively reinforce well-crafted PRs that adhere to team standards by acknowledging the author’s good work, fostering a positive team culture.
• Complete your review within the stipulated SLO. If you're busy, inform the author that your review might be delayed.

Don't:

• Approve changes that are not safe to deploy as-is.
• Request changes simply based on personal preferences for coding style. Instead, provide resources (e.g., code review guidelines) that justify your comments, linking to relevant sections of the style guide when possible.
• Make reviews personal; critique the code, not the author. For example, instead of saying, "Why are you making this more complex?" rephrase it to, "The introduction of Gevents adds complexity without performance benefits because…"
• Lock the PR unless it is clearly unacceptable for merging or poses risks. Locking is a serious action and should be approached with caution. If it is necessary to block a PR, provide a clear explanation of the issue and collaborate with the author to resolve it.

By implementing these standards, our team successfully reduced the code review time from over a day to approximately 3 hours, fostering a more positive and thorough code review culture.

Chapter 2: Video Insights on Code Reviews

This video titled "START OF SOMETHING GOOD | Reading Your Comments #1" dives into the nuances of code reviews and community feedback, offering further insights.

The second video, "READING MY COMMENTS | PART 1," continues the discussion, highlighting the importance of constructive criticism and best practices in coding.

How does your team approach code reviews? Are there any points in this article that resonate with you or that you disagree with? I welcome your thoughts in the comments below.

Feel free to follow me for updates on similar articles. If you're interested in job interviews, you might also want to read my articles titled "The LeetCode Phenomenon" or "Why Microsoft Didn't Love Me Back."