Pull Request Best Practices

A pull request is a request for someone else's time. Keep the change to one decision, keep the diff small enough to follow in one sitting, and ask a person who knows that path. The description has its own template. This is about the change you put in front of them.

Tabs, commit-message prefixes, and whether you squash belong in the linter and the repo settings. A review spent on those is a review that did not look at the behaviour.

One decision per pull request

If the description needs "and also", it is two changes. Split them. A reviewer can hold one decision. Two decisions means they approve the safe half and skim the other, which is how the risky half lands.

A refactor mixed with a behaviour change is the usual case. Ship the move first, with no behaviour change, then the change. The second diff is the one that needs a careful read, and it is short.

A size you can finish

There is no line count that makes a pull request reviewable. Forty lines in the billing job can take longer than four hundred lines of generated client. The test is whether a reviewer can follow one path from the entry point to the result without taking notes.

If they cannot, the change is too big, or you have not said which path to follow. Say it in the description. If you still cannot point at one path, split the pull request.

Ask when it is ready

A review request on a red build asks someone to comment on code you are about to change. Leave it in draft.

Ask one owner

Ask the person who last changed that path, or the owner of the service. A request sent to six people is a review nobody owns. On a small team that person may be the only other engineer. That is still one person, and a time you agreed. "Whoever is free" is how a pull request sits until Friday.