All My Best Tips For Expertly Reviewing Your Own Code
- 5 minutes read - 1007 wordsGreat developers review their own code often and effectively. In this post, I’ll share a checklist of all my best tips for maximizing this important practice.
1. 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.
2. 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.
3. 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.
4. 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.
4. 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.
5. 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.
6. Add Comments
Help your reviewers see the signal in the noise, and preempt questions someone else might raise with your own comments on the PR.
I usually leave a few comments on my own PR to document all the context I have at that moment. A few examples:
“Note: I considered not logging this to Sentry, but we have no other way to know if it fails.” “Note: this is the bugfix; everything else in this file is required auto-formatting.” “Note: 100 is a magic number but this is all about to be redesigned so I kept it simple.”
The sweet spot here are messages that help you and others figure out what matters– what you were trying to do– and explain any rough spots. These comments are too contextual and ephemeral to live on in the code as comments, but they are still pretty useful.
I try to use Conventional Comments even for my own reviews. “Note:” and “Question:” help clarify the effect I hope the comment will have.
7. Self-Review All the Time
If you have someone to review your code, why bother to review it yourself? Isn’t that redundant?
I think there’s something really pro about submitting work that has the obvious points of feedback– think typos, abandoned ideas, bad names, etc.– removed. Timeboxed, of course. You’re conditioning your teammates to expect quality. You’re teaching your reviewers that your pull requests contain substantive ideas that are ready for immediate consideration. And you’re training yourself to become a sharper and more humane reviewer by frequently putting your own work under the microscope.
Self-review as often as you can.
8. 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.