🔀 Pull Requests and Code Reviews

Why Pull Requests Exist

A pull request (PR) asks a project to pull the changes on your branch into another branch — usually main. But its real value is everything that happens before the merge: teammates read the diff, ask questions, catch bugs, and leave a paper trail future developers can read.

💡 A useful analogy: A pull request is like submitting an essay for peer review before publication. You don't just slide your draft onto the printing press — you circulate it, colleagues mark it up, you revise, and only the approved version goes out. The comments left behind also explain the reasoning long after the ink dries.

Pull requests give a team four things at once: quality control (a second set of eyes), knowledge sharing (reviewers learn the codebase), mentorship (juniors learn from seniors), and documentation (the discussion is preserved forever).

Anatomy & Lifecycle

Every PR compares a source branch (your changes, also called head) against a target branch (where you want them to land, also called base). Around that core it gathers a title, a description, the list of commits, the changed files, review comments, status checks, labels, and linked issues.

The lifecycle

A PR moves through a predictable set of states. Understanding them tells you exactly what needs to happen next:

1. Creation — opened as a draft (work in progress) or ready for review.
2. Review — reviewers read the code and leave feedback.
3. Revision — the author addresses feedback with new commits.
4. Approval — reviewers approve once satisfied.
5. Merge — the change lands in the target branch.
6. Cleanup — the source branch is deleted.

Authoring a Great PR

The reviewer's job is only as easy as you make it. Two habits matter more than anything else: keep PRs small and write a description that answers questions before they're asked.

Prepare before you open it

# Bring your branch up to date with the target
git checkout main
git pull
git checkout feature-branch
git rebase main

# Run the tests and linters locally first
npm test
npm run lint

# Review your own diff before anyone else does
git diff main...feature-branch

Keep it small and focused

A PR should do one thing. Research and hard experience agree that review quality falls off a cliff past a few hundred lines of change — reviewers skim, and bugs slip through. When a feature is large, split it into a sequence of smaller PRs that build on each other.

Write a description that helps

A good description covers what changed, why it was needed, and how you approached it, then notes any testing and risks. Standardize it with a template at .github/PULL_REQUEST_TEMPLATE.md:

## What & why
Brief summary of the change and the problem it solves.

## Related issue
Closes #42

## How it works
Key implementation decisions a reviewer should know about.

## Testing done
- Added unit tests for the new validation path
- Manually verified the happy path and empty-input case

## Checklist
- [ ] Tests added / updated
- [ ] Docs updated if needed
- [ ] No new warnings

Reviewing Code Well

A good review is systematic, not a random scroll through the diff. Work from the outside in: understand the context, grasp the big picture, then examine details.

1. Read the description and any linked issue — know what the change is trying to do.
2. Scan the overall shape — does the architecture fit the codebase before you nitpick lines?
3. Read line by line for correctness, edge cases, and clarity.
4. Check the tests — do they cover the change, including error paths?
5. Confirm the docs reflect any user-facing change.
6. Summarize your findings and choose an outcome.

What to look for

Prioritize in this order — correctness and security matter far more than a misplaced space:

GitHub's review outcomes

When you click Review changes, you choose one of three verdicts:

• Comment — general feedback, no explicit verdict.
• Approve — you're happy for this to merge.
• Request changes — something must be fixed before merging.

Use the suggestion feature (the pencil icon on a line comment) to propose exact replacement code; the author can accept it with one click.

Constructive Feedback

The tone of your feedback decides whether it improves the code or starts a fight. The golden rule: review the code, not the coder. Assume good intent, be specific, and explain the why behind each suggestion.

A well-structured review balances praise with suggestions and separates must-fix from nice-to-have:

### Strengths
- Token generation is clean and well tested.
- Nice job isolating the auth logic in its own middleware.

### Suggestions (blocking)
- The login route should rate-limit attempts to prevent brute forcing.

### Nits (non-blocking)
- Consider renaming `d` to `expiresAt` for clarity.

### Question
- Why the 24-hour token lifetime? Would a shorter one hurt UX much?

// Instead of deeply nested conditionals:
function processUser(user) {
  if (user) {
    if (user.isActive) {
      if (user.hasPermission) {
        return true;
      }
    }
  }
  return false;
}

// Prefer early returns — the "happy path" reads top to bottom:
function processUser(user) {
  if (!user) return false;
  if (!user.isActive) return false;
  if (!user.hasPermission) return false;
  return true;
}

Automating the Boring Parts

Humans should spend their attention on design and logic, not on catching a missing semicolon. Let automated status checks handle the mechanical review so people can focus on judgment.

Run checks on every PR

A GitHub Actions workflow can lint, test, and scan every pull request automatically:

name: Code Quality Checks

on:
  pull_request:
    branches: [ main ]

jobs:
  lint-and-test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: Set up Node.js
        uses: actions/setup-node@v4
        with:
          node-version: '20'
          cache: 'npm'
      - run: npm ci
      - run: npm run lint
      - run: npm test

Then make those checks required: in Settings → Branches, add a branch-protection rule for main and enable "Require status checks to pass before merging." Now a PR simply cannot merge while tests are red.

Hands-on Exercise

Best Practices

Summary & Quiz

📚 Further Reading

• GitHub Docs — Pull Requests
• Google Engineering Practices — Code Review
• Conventional Comments
• Conventional Commits

🚀 What's Next?

You've seen automated checks running on a pull request. Next we'll go under the hood of that automation and learn to build workflows from scratch with GitHub Actions.