Crafting the perfect PR

Posted on: 16 August 2023

I find myself inside pull requests a lot at work. Whether I’m authoring one myself or reviewing a co-worker’s, I’ve become pretty familiar with the Github PR screen and what makes a good PR review experience. So here’s my top tips to improve your PRs.

Base it off a solid, well-researched issue

This isn’t a hard rule, but it’s much nicer to have some separate context for why this PR exists. Your PR should contain this information (why it came to be), but the detail should be in a clear, well-written issue.

In some organisations, a pull request can’t exist without an issue, as work is scheduled based on the backlog of issues. In more relaxed environments like at Arena Flowers, I try to stick to, and encourage, the convention of issue first, PR second.

Keep it succinct

It was renowned programmer Kent Beck that stated:

For each desired change, make the change easy (warning: this may be hard), then make the easy change.

I believe this can be translated to how a pull request should be approached.

Every new feature or change of behaviour to an existing feature typically requires a change to the current codebase. It’s your job as the author of this work to present these changes in as clear a fashion as possible.

It’s tempting to plough on with your new feature, making every little change required to accomplish your goal, and swallowing up the context that the change was made into a monolithic PR. But when you take this approach, when you finally present this PR for review to your peers, you’re met with confusion, or perhaps even a wall of silence.

Every blocker you encountered, every decision you made along the way, has been combined together and presented in one big contextless chunk. And your expectation as the author is that somehow your colleagues are able to understand and follow your thought process across all these changes and give you some meaningful feedback.

There’s a better way.

Small PRs are joyful

If you can summarise your PR in a sentence, you’ve won the PR game. If you can get an insta-sign-off from a colleague, congratulations, you’ve won the PR game.

Make your PR easy to review.

Those micro changes to your existing codebase to facilitate your Brand New Feature; let them breathe in their own PR. Get them reviewed, merged and deployed. And your Brand New Feature is one step closer to being complete.

If that change happens to alter a core behaviour of your system, introduce a feature flag. You’re then setting the ball in motion for your Brand New Feature by adding flexibility to the existing system. This way when the time comes to merge and enable your new code, accommodation is simple.

Yes, this will slow down development of your Brand New Feature, but nothing slows down the release of a feature more than having it sit in review until it’s stale and no longer relevant.

The PR is your sales pitch

The PR is your feature’s early documentation. It’s the sales pitch to your team as to why this code should be added to your codebase. Sell it.

Your change might be so simple even a non technical person can see why it’s being made. Great, but document it. Explain why it’s being added. Why is it needed? What was wrong with the previous code?

A PR is always in your history, even after it’s merged. It’s easy to say that PR is done. But it’s always there, providing value when it’s needed in 5 years time when a new crop of developers have entered your team looking for context.

But the documentation should not be confined to your PR. Your PR is the genesis for this documentation, but it should be moved somewhere central and accessible for your team shortly before or after merging.

Entice your reviewers

Well written text documentation is great. But nothing sells a feature more than a series of screenshots, or better yet, a video. Invite your peers on a journey of what this PR will give to your users. Or how it might improve developer experience going forward.

Making a screen recording demoing the feature you’ve spent weeks building might feel like a pointless chore at the end, but it might be the difference between someone engaging and providing valuable feedback on your PR, and them passing on by.

This supplementary, immersive documentation can also be used as a guide for users of the feature down the line. Double benefit.

Keep it on point

If you’re distracted by a typo or a method ripe for refactor while you’re building out your feature, then your reviewers will be too.

Leave it out of the PR. Keep your PR targeted to your end goal.

By all means open up a side quest PR fixing a typo. Get it reviewed and merged and move on. But don’t let it muddy up your feature and add visual clutter to your hard work. Every line counts.

Run your automated checks

This almost goes without saying, but it’s so vital it bears inclusion. Your CI pipeline should run your automated test suite and linting checks. And indeed anything else vital to the running of your system. And obviously it should pass.

I’ve worked on enough codebases without this to know if you can make a mistake and break the build, you will (Murphy’s law). If your automated test suite fails, block merge. This is trivial to do but might be the difference between uptime and downtime!

Nothing breeds confidence like reviewing a PR with a suite of shiny green ticks next to it. You can be reasonably assured nothing breaking has been introduced, and that the new code adheres to your coding guidelines.

Get sign off

Every company will have their own internal structure for sign-off for something going live. But the key thing is to make sure the people who might be affected by your PR are notified ahead of it going live. It may not require their sign-off, but a heads-up is always appreciated. No one likes surprises.

Make it previewable

This isn’t always possible in legacy codebases and older CI setups, but ideally every PR should have its own preview link. Ideally, also, its own self-contained database.

Seeing changes with your own eyes is infinitely more likely to get a good review than just lines of code. If your changes are visual but you don’t have this ability, copy the changes and other relevant code into a website like CodePen and share this.

If your application uses Docker, or a Jamstack compatible service such as Netlify, you can relatively easily spin up an instance of your application based off the code in the PR and get a real URL to navigate to. This is incredibly useful for end-to-end and real world testing of your PR, where necessary.

Happy PRing

As an author of a Pull Request, you have a responsibility to your team and peers. With a little foresight, empathy and preparation, your team will look forward to your PRs coming into their in-trays.