Getting Started

The purpose of this document is to outline the standards and best practices for conducting code reviews within our engineering team. Code reviews are essential for maintaining code quality, sharing knowledge, and ensuring best practices are followed. This document serves as a guide to help the team perform effective and constructive code reviews.

For a more in-depth understanding of the code review process, please refer to the Starter Guide, Authors Guide and Reviewer Guide

Objectives

Improve code quality and maintainability.
Ensure adherence to coding standards and best practices.
Foster a collaborative and respectful team environment.
Share knowledge and promote continuous learning among team members.

Code Review Process

Pull Request Submission

Authors should submit their code changes via pull requests (PRs) in GitHub.

Draft PRs

Authors should mark their pull requests (PRs) as draft until they are fully prepared for review. This practice signals to reviewers that the code is still a work in progress and not yet ready for detailed feedback.

Assign Reviewers

The author should first try to assign a team of reviewers if applicable, to ensure diverse perspectives and thorough feedback. In addition, specific reviewers can be assigned based on their expertise and familiarity with the code area.

Review Timeline

Reviewers should aim to complete the review within 24 hours.

Addressing Feedback

Authors are expected to address feedback promptly. If significant changes are made, the PR should be re-reviewed.

Roles and Responsibilities

Author: The developer who submits the code for review. Responsible for providing context and answering questions raised during the review.
Reviewer(s): Developers assigned to review the code. Responsible for providing constructive feedback, asking questions, and approving or requesting changes.
Lead Reviewer (if applicable): A senior developer who oversees critical reviews and ensures standards are met.

Principles of Code Review

Purpose of Code Review: The primary purpose of code review is to make sure that the overall code health of Circle’s codebase improves.
When to approve a pull request: Approving a pull request is one of the most important responsibilities of a reviewer, and deciding when to approve a pull request can be challenging. This is one of the golden rules that we can follow - In general, reviewers should favor approving a PR once it is in a state that:
it definitely improves the overall code health of the system.
it is functionally correct. even if the PR isn’t perfect. To quote Google’s code review standard:

Instead of seeking perfection, what a reviewer should seek is continuous improvement.

Best Practices

Small Changes

Encourage small, frequent PRs to simplify the review process.

Constructive Feedback

Provide feedback that is constructive and actionable.

Respect Different Perspectives

Acknowledge differing viewpoints and foster open discussions.

Common Pitfalls

Nitpicking and Bikeshedding

Avoid getting caught up in trivial issues that don’t affect functionality, as well as spending excessive time debating on minor details (bikeshedding). Focus instead on critical aspects of the code’s logic, design, and overall effectiveness.

Many of these nitpicky issues can often be automated through the use of linters and code formatters. Ensure that your project is set up to use these tools effectively to catch style and formatting issues before the code reaches the review stage. This allows reviewers to concentrate on more significant concerns.

Subjective Preferences

Be cautious of comments such as “that’s not how I would have done it.” Recognize that there are often multiple valid approaches to solving a problem. For example, an author might choose to use a traditional for loop instead of stream operations. These differences can be trivial to refactor later, and it’s essential to prioritize functionality and readability over personal coding style preferences.

Ignoring Feedback

Authors should not dismiss feedback without discussion; engage in constructive dialogue.

Lack of Documentation

Ensure that all relevant code changes are thoroughly documented. This includes updating comments within the code to explain the purpose and functionality of complex sections, as well as any relevant documentation files, such as README.md or other project-specific docs.