Pull Request Best Practices

A pull request (PR) is more than just a way to merge code — it's a communication tool. A well-crafted PR makes review easier, catches bugs sooner, and keeps the project's history clean.

Anatomy of a Good PR

Title

Keep it short and descriptive. Use the imperative mood, like a commit message:

• Good: "Add email validation to signup form"
• Good: "Fix crash when user has no profile photo"
• Bad: "Updates" or "Fixed stuff" or "WIP"

Description

A good PR description answers three questions:

1. What does this change do?
2. Why is it needed?
3. How can the reviewer test it?

## What
Add server-side validation for email addresses on the signup endpoint.

## Why
Users could submit invalid emails, causing downstream failures in
the notification service.

## How to Test
1. POST to /api/signup with an invalid email
2. Verify you get a 422 response with a validation message
3. POST with a valid email and confirm signup succeeds

Size

Smaller PRs are better PRs:

• Small (under 200 lines) — Easy to review, quick feedback
• Medium (200-500 lines) — Acceptable for features
• Large (500+ lines) — Hard to review thoroughly, consider splitting

The Code Review Process

As an Author

• Self-review first — Read through your own diff before requesting review
• Add context — Leave comments on tricky parts explaining your reasoning
• Respond to feedback — Address every comment, even if just to acknowledge it
• Keep commits clean — Each commit should be a logical unit of change

As a Reviewer

• Be constructive — Suggest improvements, don't just criticize
• Ask questions — If something is unclear, ask rather than assume
• Approve when ready — Don't block on nitpicks; note them but approve if the core change is sound
• Check the big picture — Does the approach make sense? Is there a simpler way?

CI Checks

Most projects run automated checks on every PR:

• Tests — Does the existing test suite still pass?
• Linting — Does the code follow the project's style guidelines?
• Build — Does the project compile/build successfully?
• Coverage — Is the new code covered by tests?

A green CI status means the automated checks passed. Always wait for CI before merging.

Merge Strategies

When a PR is approved, there are several ways to merge it:

Merge Commit

git merge --no-ff feature

Creates a merge commit that preserves the full branch history. Good for seeing when features were integrated.

main: A --- B --- M
               \ /
feature:  C --- D

Squash and Merge

Combines all PR commits into a single commit on the target branch. Good for keeping main history clean when PR commits are messy.

main: A --- B --- S  (S contains all changes from C and D)

Rebase and Merge

Replays each PR commit on top of the target branch. Good for a linear history while preserving individual commits.

main: A --- B --- C' --- D'

PR Workflow Summary

1. Create a branch from the latest main
2. Make small, focused commits with clear messages
3. Push your branch and open a PR
4. Write a clear description explaining what, why, and how to test
5. Wait for CI to pass
6. Address review feedback promptly
7. Merge using the team's preferred strategy
8. Delete the branch after merging to keep things tidy

Common Pitfalls

• Giant PRs — Break large changes into smaller, reviewable pieces
• No description — Always explain what your PR does and why
• Ignoring CI failures — Fix them before requesting review
• Long-lived branches — Merge or rebase frequently to avoid painful conflicts
• Merging without review — Even small changes benefit from a second pair of eyes