Example of Splitting a PR

May 25th, 2025
tech
I'm a big fan of small focused PRs: it's much easier to tell whether they're actually doing the right thing. For example, I recently ran into a common scenario where I started adding some functionality, and realized this required some refactoring. The refactoring also made it clear that the original code was missing some important test coverage, so I added some tests. I ended up with a big set of changes which combined large refactors and a major (intended) change to the output.

This is a bad combination: if my refactoring accidentally changed functionality in a way not captured by the tests we might miss it. And with so many changes it's going to be hard for the reviewer (and even the author) to keep it all in their head at once. Instead, it would have been better if I had first done the refactoring and added tests, and then followed up with the new functionality. The first PR would be a bit noisy but conceptually straightforward and easy to validate because it produces exactly the same output for all inputs. The second PR would be very clean, but would require careful validation to verify that the changes really are an improvement.

I might be tempted to say "oh well, I should have approached this differently, I'll do better next time" but really I did need to combine these two changes when writing them: I didn't know for sure what I would want for the refactoring until I'd tried to use it to implement the functionality change. So if my plan was to do better next time I wouldn't actually do any better.

Instead I took advantage of version control to split my changes into two sets, in an order that is optimized for validation:

  1. I committed all my changes to a new branch, jefftk-new-feature, with a message that only mentioned the feature I was adding, not the refactoring or the new tests.

  2. I made a new branch off of jefftk-new-feature to represent the refactoring and new tests: git checkout -b jefftk-refactor.

  3. On jefftk-refactor I deleted everything related to the new feature, leaving only the refactoring and new tests. I verified that the output was unmodified, as you'd expect for a refactoring-only change. I committed this, with a message that only mentioned the refactoring and tests.

  4. I interactively rebased (git rebase -i HEAD~2) this branch to squash the two commits into one. I only kept the refactoring commit message. This branch is now ready to be my first PR, and is ready to send out for review.

  5. I switched back to jefftk-new-feature, and ran git rebase jefftk-refactor. This rewrote my original big commit as if it was a follow-up to the refactoring commit. There were a few merge conflicts I needed to resolve manually, but nothing too painful. This branch is now ready to be my second PR, and I'll upload it to GitHub so I can reference it in the first PR in case the reviewer is interested in taking a peek at what's coming.

This was a bit more work, but I think it's definitely worth it: I'm much more confident that my refactoring didn't break anything, and I expect it to be much easier to review.

Comment via: facebook, lesswrong, mastodon, bluesky, substack

Recent posts on blogs I like:

The One That I Want Another Sister

I want a baby sister or brother. Because I want to play with another kid, and I don't want to play with Josie because she is in preschool and I won't play with her very often. So I want to play with someone very often who is not just two people. …

via Nora Wise's Blog Posts September 5, 2026

Linkpost for September

On September 19 and 20, Sparrow Infrastructure is putting on The Overhang, a conference in Washington D.C.

via Thing of Things September 4, 2026

Ed Zitron's AI prediction track record

I was curious how well the predictions of the most widely cited AI skeptic I've seen (Ed Zitron) have done, so I looked at how his predictions panned out. To disclose my own biases, I've never had a particularly strong pro or anti AI progress posi…

via Posts on September 1, 2026

more     (via openring)