Code Review Process

Code reviews are a critical practice to ensure code quality, share knowledge, and foster collaboration. Every Pull Request (PR) must be reviewed and approved by at least one other developer before it can be merged.

The Author’s Responsibilities

  1. Self-Review First: Before you request a review, review your own code.
    • Does the code work and meet the acceptance criteria?
    • Is the code clean, readable, and consistent with the style guide?
    • Have you added necessary tests?
    • Is the documentation (e.g., README, comments) updated?
  2. Create a Clear Pull Request:
    • Write a descriptive title and a clear summary of the changes.
    • Include a link to the relevant task in the project management tool.
    • Provide testing instructions and screenshots/GIFs for UI changes.
  3. Keep PRs Small: Small, focused PRs are easier and faster to review. Aim to address a single feature or bug fix in each PR.
  4. Respond to Feedback Gracefully:
    • Be open to constructive criticism. The goal is to improve the code, not to criticize the author.
    • If you disagree with a suggestion, explain your reasoning calmly and professionally.
    • Use comments and threads to discuss feedback.

The Reviewer’s Responsibilities

  1. Be Timely: Aim to review PRs within 24 hours of the request. If you can’t, let the author know.
  2. Understand the Context: Read the PR description and the linked task to understand the purpose of the changes.
  3. Provide Constructive Feedback:
    • Be respectful and empathetic in your comments. Frame feedback as suggestions, not commands (“What do you think about…?” instead of “Do this…”).
    • Focus on the code, not the person.
    • Balance criticism with praise. If you see something done well, say so!
  4. Check for…
    • Correctness: Does the code do what it’s supposed to do? Does it handle edge cases?
    • Readability & Maintainability: Is the code easy to understand? Is it overly complex?
    • Consistency: Does the code follow our coding style guide?
    • Testing: Are there sufficient tests? Do the tests cover the important cases?
    • Security: Does the change introduce any potential security vulnerabilities (e.g., XSS, CSRF)?
  5. Approve or Request Changes:
    • If the PR is good to go, approve it.
    • If there are issues that need to be addressed, select “Request Changes” and provide clear, actionable feedback. Avoid approving a PR with comments like “Looks good, but please fix this one thing.”

Code Review Etiquette

  • Automate What You Can: The linter and formatter should handle stylistic issues. The code review should focus on higher-level concerns like logic and architecture.
  • Use “Nitpicks”: For minor, non-blocking suggestions (like a typo in a comment), prefix your comment with “Nit:” to indicate that it’s not a major issue.
  • In-Person/Sync Discussion: If a discussion in the PR comments is going back and forth, it’s often more efficient to have a quick call to resolve the issue. Summarize the outcome of the discussion in the PR comments for a record.