What Makes a Good Pull Request in a Ruby on Rails Codebase

Code review quality has less to do with how carefully reviewers read and more to do with how the pull request was put together in the first place. Here's what we look for, and what we ask engineers to change, when we join a Rails team as a staff-level contributor.

Small enough to hold in your head

A pull request that touches a dozen files across three unrelated concerns forces a reviewer to either rubber-stamp it or spend an hour reconstructing the intent. We aim for changes that do exactly one thing, and split refactors from behavior changes into separate PRs even when it's tempting to bundle them.

A description that answers "why," not just "what"

The diff already shows what changed. What it can't show is why this approach over the obvious alternative, what was tried and rejected, and what the reviewer should pay closest attention to. A two-line description saying "fixes bug" gives the reviewer nothing to check against.

Tests that would have failed on main

The single most useful thing a reviewer can do before reading a line of implementation is check whether the added test actually fails against the old code. A test that passes before and after the change isn't testing the fix, it's testing that the file compiles.

# Prove it first: temporarily revert the fix locally,
# run only the new test, confirm it fails, then reapply the fix.
bundle exec rspec spec/services/invoice_charger_spec.rb

No drive-by refactors

Renaming a variable, reformatting a file, or "cleaning up" an unrelated method inside a bugfix PR triples the surface a reviewer has to check and makes the eventual blame history useless for the next person debugging that line. If it's worth doing, it's worth its own PR.

None of this requires tooling. It requires treating the pull request itself as a deliverable, not just a container for a diff.