GitHub Pull Requests

Overview

This guide will help new developers understand how to author, merge, and review pull requests (PRs) within our organization’s repository. It covers the merging process, and best practices for both authors and reviewers.

Authoring a Pull Request

Creating a Pull Request

1. Open a PR whenever you want to share your code:

   • You can open a PR at any stage of your work.
   • If your PR is not ready for review, mark it as a draft and do not assign reviewers.

2. Clear Intent for Draft PRs:

   • Indicate if a draft PR is ready for preliminary review despite being marked as a draft.

3. Commit Management:

   • Once a review has started, avoid rebasing, squashing, or amending commits.
   • Do not rebase onto a more recent target unless necessary. Merge the target branch into your PR branch if needed.

Addressing Feedback

1. New Commits for Feedback:

   • Address review feedback by adding new commits.
   • Keep these commits focused and narrow in scope.
   • Let reviewers mark feedback as resolved.

2. Use GitHub UI:

   • Prefer the GitHub UI for responding to feedback for better context and to avoid missed comments.

3. Cleaning Up:

   • After approval, you can squash or rebase to clean up commit history if necessary.

Merging Process

Using Mergify

• Automated Merging:

  • We use Mergify to manage the merging process.
  • Mergify is subject to the same branch protection rules and is triggered by specific labels.

• Avoid Manual Merge:

  • Do not use the GitHub UI merge button. Use the automerge labels instead for fairness and consistency.

Branch Protection Rules

• Required Approvals and Checks:
  • A PR must be approved by at least one reviewer.
  • All required status checks must pass.
  • The PR must be up-to-date with the target branch.

Reviewing a Pull Request

As an Author

• Reviewer Assignment:

  • Assign reviewers when the PR is ready.
  • Avoid assigning too many reviewers and be clear about the scope of the review.

• Commit Handling:

  • Add new commits for changes instead of amending existing ones.
  • Indicate feedback consideration using comments or emojis.

As a Reviewer

• Comprehensive Review:

  • Review the code, comments, PR description, and commit structure.
  • You share ownership of the code with the author once you approve.

• Feedback Classification:

  • Fix: Must be fixed and re-reviewed.
  • Fix & Ship: Must be fixed but no need for another review.
  • Optional/Nit: Suggested changes at the author's discretion.
  • Question: Clarification needed.

• Constructive Feedback:

  • Provide actionable feedback with a clear description of issues and suggested fixes.

Best Practices for a Good PR

• Small and Targeted:

  • PRs should be focused and linked to relevant issues.
  • Include a complete PR description and a clear title.

• Test Coverage:

  • Ensure that tests cover the changes made.

• Ease of Review:

  • Break changes into small, logical commits.
  • Avoid mixing refactoring with behavioral changes.

PR Style Guide

Merge Strategies

• Automerge Labels:
  • Use automerge:squash for small, single-commit PRs.
  • Use automerge:rebase for larger PRs.

Commit Messages and PR Titles

• Conventional Commits:
  • Use clear and descriptive commit messages and PR titles.
  • Follow the conventional commit format if applicable.