Soloists: code review on a solo project

My development process leading up to code review

Code review is just one step in the development process. I find code review is easier and less risky if I follow these rules:

1. Research, think, and plan as much as possible before coding.

2. Talk to myself about what I'm doing. A lot.

3. Keep changes small.

   The larger a change is, the longer it takes to review and the higher risk of introducing bugs and failures.

   Sometimes I'll break up changes into a set of smaller atomic changes. Sometimes forcing myself to think about breaking big changes into small atomic changes makes it easier to do big changes.

My self-review flow

I think those three things create a context in which it's easier to do self-review.

For self-review, this is roughly what I do:

1. I run all the linters and tests I have set up for the project.

   This covers project conventions and makes sure everything is good as far as computers can tell. Forcing computers to do work means I do less.

2. I go take a walk and do something somewhere else for 15 minutes.

   This is a good time to get a glass of water [3]. I work at home, so sometimes I'll spend some time decluttering or cleaning.

3. I look at the changes in different ways using different tools.

   When I come back, I try to look at the code changes in a bunch of different ways and see if it looks ok.

   I do:

   git diff master..HEAD

   or something equivalent in the terminal and read through my changes carefully. git has a variety of different diffing and ways to view. For example:

   git diff --histogram master..HEAD
   git diff --minimal master..HEAD
   git diff --patience master..HEAD
   git diff --word-diff=color master..HEAD

   Definitely worth reading through git diff --help to see what would help you.

   I may also look at each individual commit and the commit comments and summary.

   I may look at the original code in full (checkout the master branch) and then look at the changed code in full (checkout the branch tip).

   Looking at the changes using different tools and views helps force my brain to be more careful reading because it looks totally different. Different tools use different diffing algorithms, have different fonts and color schemes, and show different amounts of context.

   This sort of covers developer best practices, programming language/library best practices, and things that require someone who's been working on the project for a while.

4. Fourth, I push my branch and create a pull request on GitHub.

   The CI system I use (Circle CI or Travis CI depending on the project) happily runs all my tests and linters.

   If there are CI failures, I'll fix them.

5. I leave it for a day.

   I'll do other things and ignore the PR until the following day. A night of sleep between creating the PR and reviewing it gives me time to mull over things and helps me forget bits.

   Sometimes I get side-tracked on something else--I usually have multiple projects I'm working on, so I'll switch between them.

6. I review my pull request.

   I go through the pull request carefully and answer questions like the following: questions:

   1. How can this code fail? What have I done to prevent those failures? What have I done to surface those failures so I know if they're happening?

   2. What happens if this code is wrong? Will it cause data loss? Will it break the service-level agreement I have with my users? Can this change be backed out quickly if it's bad?

   3. Are these changes appropriately covered by the tests? Do the tests make sense?

      If I'm using mocks in my tests, have I verified that the mocks are correct?

   4. Do the changes match the documentation and surrounding comments?

   I take my time reviewing my pull requests. I don't rush it. If I'm tired, I take a break.

   I write comments to my PRs especially to explain design decisions that I think might look odd. Future me and any other developers that join this project will benefit greatly from me explaining why I did what I did. Also, this is sort of like rubber-ducking and I sometimes discover things I should change because they're either hard to explain or it becomes clear it's missing edge cases.

   This sort of covers developer best practices, programming language/library best practices, and things that require someone who's been working on the project for a while, too.

7. I land the change!

   After all that, I'll land the change. If there are issues, I'll fail forward and fix them if I can. If I can't do that, I'll back it out. I don't think I've ever backed anything out, though.

That's pretty much it.

Periodically, I revisit code a couple of weeks or a month later and see if it still looks ok. Sometimes code doesn't evolve well over time and is worth refactoring.

Do I do all these steps every time?

No. I try to balance the urgency of the change to land with the risks of failure with the consequences of failure and base how much time I should spend reviewing on that.

For example, if I'm fixing documentation, I won't spend a lot of time reviewing it. If I'm fixing a critical method in a piece of code that needs to be thread-safe and never fail, then I'll take a couple of days and be more meticulous.

Warning: Self-review doesn't help you build people or build yourself!

In a self-review, you're both the developer and the reviewer. That bites you in both directions.

When you review other peoples' code, you share your experience and knowledge with them and help them grow as a developer and grow as a contributor on your project. If you're self-reviewing, you're not growing anyone.

When someone else reviews your code, they share their experience and knowledge with you and cover the things that are unknown to you. Thus code review is a unique opportunity to learn from others and grow as a developer. If you're self-reviewing, you're not growing yourself.

If you're self-reviewing, you need to work harder to alleviate these missed opportunities.