What Ugly Pull Requests Look Like

Jamie Wen
2 min readFeb 23, 2022

Things that need to be avoided for a better dev experience

There are tons of best practices for PRs are out there for reference, like Write a good pull request, The unwritten guide to pull requests, etc. This makes me curious about what ugly pull requests look like. Let’s dive in.

Ugly Pull Requests don’t tell the What and the Why

Empty description, not even a word. To be fair, if the PR title explains everything already, that’s fine. Otherwise, think about what kind of information you want to send to your peers, your future self, new team members, and the poor guy who will work on this project 2 years from now. Do them a favour, please. 🙇 🙇‍♀

Ugly Pull Requests do more than one thing

Anti Single Responsibility. Ugly PRs try to solve multiple issues in one go but fail to meet the criteria.

The goal of an ugly pull request is to be overly ambiguous. I am going to upgrade packages, reformat files, refactor some util functions, change build steps, and most importantly, deliver a big feature. No, you can’t. No one can get it done in one go. Split them into multiple PRs and ship them individually.

Do this ⛳️⛳️⛳️️⛳️️⛳️️

Don’t do this 🕳

Ugly Pull Requests are complex

Code reflects the complexity of the requirements, and that’s not uncommon. However, ugly pull requests reflect the complexity of the predicted (imagined) requirements. Surely we need upfront design, but

Upfront Design != Over Engineering

Ugly Pull Requests have no screenshot when it’s needed

The diff shows me lots of CSS changes, but no screenshots are provided in the description. Unfortunately, I am not smart enough to process the DOM/Styling in my brain to visualise the changes.

A picture is worth a thousand words

Ugly Pull Requests have errors when sent for review

Maybe the PR isn’t ready for someone else to review? Maybe you should raise it as a Draft PR first, and ensure it passes all of the checks? Maybe you should respect people’s time?

Ugly Pull Requests take a longer time to merge

The longer it takes, the higher the risk of failure. Again, keep it small.

Ugly Pull Requests have endless conversations

It’s probably a sign of prematurity.

--

--