How can we create a pull request (PR) that reflects our thought process and intention?

When creating your first ever pull request, it can seem daunting. You might think: ‘What will others think of my code or changes?’ There are a few things we can do to better improve our chances of a timely and well intentioned review.

• Ensure there are tests
• Have tests pass before committing
• Lint changes before committing
• (Optional) have commits tell a story
• Your changes should be small

Let’s address some of these more in depth.

Ensure there are tests, at the right level of abstraction

We want to have tests for our change, as it helps increase the confidence in our changes. Everyone makes mistakes, but tests help us catch these. They also improve the ability for us to refactor code in the future.

If you have to ask yourself if you need tests, you likely do, at least at some level of abstraction. We want to test the right things, at the right level of abstraction. This means writing an assortment of unit tests, integration tests, end-to-end tests.

Have fast, accurate, resilient tests. Slow tests impact others and your overall test suite run time. No one likes flaky tests, so it’s important that they aren’t brittle either. Tests that flake increase the likelihood that you’ll have to fix it in the future. And you’ll definitely have to fix it when it flakes right before a hot fix.

Have tests pass before committing

The entire test suite should already be passing before we commit, so that we can reduce the amount of commits we have like ‘make tests pass’ and ‘fix test bug’.

Lint changes before committing

We want less commits like ‘fix linting’ or ‘increase linting count’.

(Optional) have commits tell a story

Some want the commits in their branch to tell a story, so that each commit builds upon the next.

Your changes should be small

Changes that are large (> 250 lines) are more difficult to reason about. Having a smaller changeset helps reviewers who’ve dedicated their time to reviewing. I’ve found that the larger the changeset, the more likely the review quality goes down. It’s harder to reason about a change when the line count and complexity is higher. This means we can’t review everything, or understand how it works, and start to ‘zone out’

Once we’ve checked off some, if not all of these, it’s time to create our pull request.

Having a good, succinct title helps with setting context for the PR. Diving into the description, we want to link tickets or issues if they apply. This lets the reviewer also check out the ticket/issue for more background if they need it. Explaining the thought process behind this PR and what we did to reach our goal. What edge cases and error handling have we considered? Sometimes I like to attach a video recording of the changes in action. This helps the reviewer see the final result if they can’t visualize it.

Be sure to respond to all of the review comments, and make adjustments where needed.

Review

When reviewing I like to look at everything mentioned above. It’s important to be respectful of the reviewer. The purposes of code review are to refine changes and reduce defects. This also means that we shouldn’t block a PR if we have nothing to to contribute. If that’s the case, we should either pass it for review to another reviewer, or approve.

Taking time to review is important. Set aside anywhere from 30-60 minutes, depending on the line count and complexity of the changes. If the time to review goes over 60 minutes per review, that’s a good indicator to break up the PR into smaller ones. Many smaller PRs are easier to review, approve, and merge in the same time as a large one.

Using positive, inclusive language when pointing out defects is important. We are after a common goal of improving this code base, and the focus should be on the code, not the reviewer. Consider using language that focuses on the changes and suggestions on what to do. Don’t say that something is outright wrong, which isn’t constructive.

Finally, we want to ensure that the pull request follows all standards. It should also follow principles such as DRY (Don’t Repeat Yourself) and SOLID (https://en.wikipedia.org/wiki/SOLID), where needed. There comes a time where we realize that principles are not hard requirements, but just items that we should consider when we’re looking at, and creating, code.