1. Be kind

We are all learning. It is often easy to forget that when one has been hyper-focused on a specific code base or area. The memories of struggles one had to overcome to reach such expertise faint and expectation of pull request authors may become unbalanced.

Most people will do their best before putting their work out there for others to scrutinize. Critique, but kindly. Especially to new people.

2. Focus on what matters

Don’t role-play the compiler. There is plenty that can be easily automated, like code style or static analysis. If you strongly feel about commenting on which line a bracket should be, or how decade long research proves that spaces are superior to tabs, consider how much effective it is to just let the compiler or a build tool make all pull requests bend to your stylish prerogative.

That shows my opinion when people leave only style comments on a pull request. Hyperbole aside, code style is not important, code style consistency is. It is much easier to win an argument when you are the compiler (or a tool in the CI process). Therefore, don’t spend your energy in style comments, but on adding tools to enforce a style consistently.

One exception I would take is on code bases where there is little return on investment on such style enforcement tooling. And the pull request submitted for review is in such blatant violation of the prevalent code style that not mentioning the discrepancy in the review would be awkward!

Reviewing code is much like critiquing a movie or a book. There is a bit of personal preference and technical commentary. The experienced reviewer will try their best to maximize technical commentary and minimize the ones related to personal preferences.

3. When demanding, explain

When it is required to demand changes as part of code reviews, provide the rationale behind the ask. I believe this is helpful two-fold. First, you, the reviewer, may be wrong. Providing a rationale gives the code author the understanding needed to contribute to the discussion and push back when appropriate, rather than obliging to the request just out of the comment resolution inertia. Secondly, it provides the context required for code authors to proactively review their own code for similar applicable feedback elsewhere in their changes - and in subsequent updates.

4. Extraordinary claims require extraordinary evidence

Said Sagan. Not so extraordinary claims require evidence too! If you are making a comment on “how bad this approach is” or “how bad of an idea it is to do this”, the best way to avoid sounding arbitrary is to expand on principle 3 above and provide evidence of the claim.

Maybe it is a link to a blog post, like the one I wrote about IDisposable pattern in C# after too many comments in different pull requests asking me to implement the unnecessary pattern (in those specific cases), maybe it is a link to the applicable public documentation or a small code snippet settling the case.

5. Comment for consensus

When I review code for code bases that I am responsible for, my main goal is to sign off on the greatest amount of proposed changes that add net value to the code base (defining value for a code base needs a completely separate blog post!).

If I can do that without having to write any single comment in a pull request review, wouldn’t that be great? After all, what is best than a pull request so good that all you can do about it is just approve it?

People working around code bases tend to create a sort of implicit culture. “We need to add unit tests for any new changes”. “You forgot to use the imperative form on your first commit message”. Along with so many other things that people working on that code base just do, because it is what is expected to get pull requests approved in that group.

Ultimately, I believe these principles help build consensus over time, and therefore a more efficient and pleasant experience for both reviewers and authors.

What do you think - what are the five principles for code reviewers that you follow?