Thinking About Code Review

Think about your team workflow. Then try what makes sense for your context. Then adjust. The most exciting words in engineering are, “It turns out…”.

A recent Twitter kerfuffle about the pull request team workflow happened to mention me. The last thing I want to do is to participate in another pissing contest about code reviews, but here I am.

Uberto Barbini @ramtop

"Why you're not using PRs in your project?" "Because we prefer to use CI/CD" "But but..." "Listen, @KentBeck invented CI, @davefarley77 invented CD. They both agree that PRs and feature branches are incompatible with CI/CD. What do you know that they don't?"

Before we go any further, please note that I don’t have anything to sell here. I don’t win or lose anything if you do or don’t have reviews, or issue certain Git commands in a certain order, or call your flow “continuous integration” when it isn’t. I’m a big fan of you working in a way that meets your needs, pursuant to “helping geeks feel safe in the world”.

Jobs To Be Done

What effects are code reviews supposed to have on team software development?

• Reduce behavioral errors.

• Reduce structural errors.

• Improve team understanding.

• Accelerate learning.

• Reduce team risk.

Principles

A bunch of folks are making changes to the same system. In what order & at what granularity should they separate, work, & integrate?

• Flow. All else being equal, smaller batches of value more frequently are worth more than larger batches of value less frequently.

• Rowing. We’re all in the same boat, so it doesn’t matter how fast one person can row, it matters how fast we can row together.

• Overhead. Every increment costs, so have as few increments as possible. (Yes, this contradicts the Principle of Flow. It’s complicated or we wouldn’t see fights about it.)

• Alignment. Align authority (the ability to take action) with responsibility (consequences flowing towards power).

• Integration. It’s not divide & conquer, it’s divide, conquer, & integrate. Account for the cost of integration. Including mistakes.

Incentives

One aspect of code review that folks seem to discount is the incentives embedded in various styles. For example, here is one dynamic affecting the pull request model:

Larger pull requests lead to longer delays which lead to more time to make changes which leads to larger pull requests. (There are inhibiting loops that push pull requests smaller, but this reinforcing loop is still in place.)

As with all reinforcing loops, we can reverse this loop by finding a place to influence it. For example, by reducing the delay for review we have less “extra” time for development leading to smaller pull requests leading to less delay.

My point here is that any team flow we choose works in a system & that system creates incentives. Choose the team flow that creates the incentives you want & avoid the perverse incentives you want to avoid.

Dimensions of Variability

An under-explored aspect to team flow is the dimensions of variability. I’ve got two:

• Blocking

• Synchronous

One dimension is blocking versus non-blocking. Can the change go to production without a review?

One part of the answer to this question is the reversibility of the change. If, like most structure changes, the change is reversible, the value of blocking shrinks. Likewise, if the probability of an adverse behavior change (or “bug”) is low enough, then the value of blocking shrinks.

The second dimension that I think matters is whether review is synchronous or asynchronous. Once you “complete” a change, does review happen immediately, some time later, or (as with collaborative development—pairing/ensemble) during the change?

I think there is fruitful space to explore in the bottom right. What if you had something kind of like NewsFeed, but for all the changes to the system. If change happened to a part of the system you weren’t terribly interested in you might get a high-level summary of the change. If the change was likely to interfere with a change you were making, then you’d be notified in detail & immediately. (I have a plan for implementing this if you’re curious.)

Conclusion

Pick the style of review that matches your context. Invent a new one if necessary. Don’t just copy the open source pull request model just cause. Think. Experiment. Share.