Well, can you blame them?
Code reviews might be the only thing in software engineering that is universally considered “good” and “the right thing to do”. Furthermore, it is one of the few things that are empirically proven to improve our software .
And yet, I oftentimes hear particular about reviews. Nobody complains about the concept, but a lot of people (I am tempted to say “everybody”) are not 100% satisfied with the implementation.
The most common compain I have heard is about the lack of timely reviews.
In this article, I will try to answer the question in the title – why some diffs are neglected while others get more attention. And more importantly – what can the author do to make his code a “bestseller”.
Note that I will be using diff and pull request interchangebly throughout the article. Also, I assume there is already a code review practice in your team. If there isn’t – this article will not be particularly useful and you must first establish code reviews before making them better.
Top 3 reasons why people avoid reviews
My insights suggest that there are 3 top reasons why people tend to avoid reviewing particular diffs:
- Volume. Not surprisingly, since large code changes require lots of time and focus to review thoroughly (sometimes more than it took to write them). Many engineers use time between meetings, morning coffee or some other convinient slot to do reviews.
- Complexity. Very similarly to volume, code with high complexity requires time and mental energy to review properly. It is of high density – a small piece of complex distributed problem solution has the same “mass” as a much larger mundane diff.
- Lack of context. If you are not familiar with a piece of code and the reason behind the changes, you cannot actually review it. Living linters, although not uncommon, don’t bring much value to the table – the purpose of the code review is to verify the correctness of the proposed change above anything else.
- Value. It wouldn’t be a top 3 without a number 4, would it? Engineers are particularly lazy (in a good sense). If your diff doesn’t bring value or it is not clear why it is important, people will tend to neglect it.
How to make your pull requests better?
The heart of my advice is simple – prepare your diffs for review with empathy.
Empathetic writing is a well-established paradigm, one where you constantly try to be in your reader’s shoes. You can start exploring it on your own – there are tons of materials (e.g. https://www.grammarly.com/blog/empathy-writing).
Start by asking yourself:
- Who is going to review my diff?
- What is her background?
- What context does she have for the given task/problem/code?
- What is the best/easiest way to read through the changes?
Every communications training urges you to think about the audience. While some may argue that you write code for the compiler (I strongly disagree with this statement), nobody can deny that the intended audience of a pull request are software engineers.
Think of ways you can help or guide your reviewers. Here are some things I like to do:
- Discuss a change in advance whenever possible. Before you have written a single line of code the level of ego is at its lowest and you can really benefit from a fresh perspective. This also gives the other engineer(s) a sense of co-authorship which is priceless during the review process.
- Review your own code before giving it to somebody else. If you are not already doing this, you will be surprised how effective a self-review might be.
- Start with why (Simon Sinek reference intended) – explain why your diff is important. Don’t forget to be empathetic with your reader – who is she, would your explanation make sense for someone without your context, how is the change important to her, etc.
- Write a short summary, explaining the entire diff so that the reviewer can read your code while keeping the general idea in her head. I sometimes use this to guide the reviewer to read the files in a particular order that makes most sense. Avoid “retelling” your code (e.g. explain why you are committing the transaction at line 356 rather than stating you are doing it).
- Give enough context of the problem and code/components under review. Try not to give too much. You can put the ticket (e.g. Jira), but, please, don’t stop there – I have rarely seen tickets that are good enough as just references. Extract what is important from the ticket and put it in the pull request description.
- Give references to documentation, especially if understanding the change requires deep knowledge about some library/third-party component. This has the added benefit of making you look at what the documentation is actually saying and verify some of your assumptions.
- Don’t be clever. A subject of its own article, clever is not the same as smart and being clever in your code is rarely good for your reader.
As you can see, all things revolve around thinking about your reviewer saving her precious time. Also, in order to make the best out of your reviews, you must start thinking about them early.
I hope you don’t treat this article as a checklist, but rather as an inspiration – ask yourself questions, be empathetic with the readers of your code and pull requests, think of ways you can help them help you.
- The impact of code review coverage and code review participation on software quality: a case study of the qt, VTK, and ITK projects by McIntosh, Kamei, Adams, Hassan
- Peer Reviews in Software: A Practical Guide by Karl Wiegers
- Code Review at Cisco Systems by SmartBear
- The CL author’s guide to getting through code review by Google