Jake Worth

• github

This post was inspired by the following Tweet:

What’s your advice for juniors when reviewing their own PRs?

What are all the things you should be looking for?

— Brandon 🚀 Flightcontrol (@flybayer) November 7, 2022

You need a code review, but you don’t have another person.

Maybe you’re building a startup, and there’s nobody else with the context to give you feedback.

Maybe your preparing a demo, and want to refine your code before sharing it with others.

Maybe you just want to get better at programming by critiquing your own work.

First, bravo! Many developers don’t review their code at all. Doing so shows you’re committed to quality.

Great developers review their code often and effectively. In this post, I’ll share a checklist of six tips for maximizing this important practice.

Throughout, I’ll be referencing GitHub pull requests, or PRs, a common medium for reviewing code. Apply these tips to however you review code.

Wait As Long As You Can

Wait as long as you can before reviewing your code.

In On Writing, Stephen King recounts that when he finishes a first draft, he puts it away and doesn’t look at it for a long time. The closer we are to our work, the harder it is to see its flaws.

So, get some distance. Even ten minutes helps. I like to open PRs at the end of the day and self-review them first thing in the morning.

Change the Medium

Change the medium; if you wrote the code in a text editor, self-review it in the browser on GitHub.

This one never fails to surprise me! When I self-review my PRs online, I never fail to notice something new. I’m not sure the psychology behind it, but it feels like my eyes gloss over questionable code when viewing it in the same place that I wrote it. The fresh presentation helps me see the work in a different way.

Ask “What Doesn’t Belong?”

To quote my colleague Josh Branchaud: “look for what doesn’t belong.”

What’s in the PR that shouldn’t be? This could be logging and debugging statements, abandoned ideas, and noise that distracts from the precise thing you’re trying to do.

We’ve all seen a PR that adds a new feature while also removing a single line of whitespace from a random file. As I wrote about in ‘When Should I Not Refactor’, these random cleanups often do more harm than good.

A good gut check for this step is: “would I want to build on top of this PR right now, live with these choices, and follow these conventions?” Listen to your instinct.

If it doesn’t belong, get rid of it.

Consider Scope

When you review your changes, do you feel overwhelmed by the scope? If so, break them into smaller pieces.

There’s a saying about reviewing code: “Show me ten lines of code and I’ll find ten improvements. Show me 100 lines… looks good!” It’s hard for anyone to give meaningful feedback on a huge PR. They are a burden on the team. Help others help you by cutting scope.

Consider Names

Names are one of the hard problems in computer science, and very important. Make them good.

Do your names make sense? Do they reflect the language, framework, and team conventions you’re working with? If you saw the same variable names in someone else’s code, what would you think?

Make renames easy, consider your names, and change them when beneficial.

Consider PR Quality

Does your PR, as a whole, makes sense? Titles like “fixes stuff” or “I did a thing” and empty PR descriptions don’t scale beyond one person. Here are some ways to know if your PR is comprehensible:

• Does it answer what, why, and how? What are you doing, why, and how?
• Does it include a link to a ticket or story, or design files you’re following?
• Does it include references to prior art, like other open, closed, or merged PRs?
• If visual, would it be easier to provide feedback with a screenshot?

Your PR should have these things because they make it understandable and discoverable. Sure it’s in the Pivotal ticket now, but lots of teams switch from Pivotal to Jira to Asana to Trello and back again. Why not take a minute and explain your work right next to the code? You’ll thank yourself later.

Add Comments

Preempt questions someone else might raise with your own comments on the PR.

I usually leave a few comments on my own PR to cement all the context I have at that moment. An example:

“Note: I considered not logging this to Sentry, but we have no other way to know if it fails.”

That’s not a great comment to put in the code, but I shouldn’t throw it away completely. It’s useful information that explains a choice I made.

I try to use Conventional Comments even for my own reviews. “Note:” and “Question:” help clarify what I’m trying to achieve with the comment and avoid low-value additions.

Self-Review All the Time

The next level is to review your own PRs all the time, even when they’re going to be reviewed later by someone else.

I always self-review my work before showing it to a peer. No matter how closely I review the code in my text editor, seeing it on GitHub always helps me see at least one thing that doesn’t belong, is out of scope, has a bad name, etc. I want my peer reviews to be substantive, which means removing the rough edges first, by myself.

Extra Credit! Squash your Commits

An extra touch is to squash your Git commits. Some people read code commit-by-commit and it can be useful information when well presented.

With the rebase tool in your belt, you can convert your commit history from a mess of “trying this”/“one more time” messages into a tidy manifest of changes that make you look very smart. It’s a little thing! And yet, when I see it, I’m primed for a positive reviewing experience.

Wrapping Up

To summarize, here’s my checklist:

• Wait as long as you can to review
• Change the medium
• Ask “What doesn’t belong?”
• Consider scope
• Consider names
• Consider PR quality
• Add comments
• Self-Review all the time
• Extra credit! Squash your commits

Thank you to Josh Branchaud and Dillon Hafer for thinking about this with me.

What are your thoughts on this? Let me know!