Effectively review pull requests

We all would have struggled at least once to get someone to review our Pull Requests (here on let’s call it PRs). When you request someone to review your PR, they take a few decades to approve 😝.

The cause for the delay is often because the reviewer is discovering the choices you took to solve the problem for the first time. There are debates about why a particular decision was taken. Sometime they might not like the way the code “looks”. We could go on and on.

Because of this, there is a lot of uncertainty on when the PR would be approved. Thus, it delays when your users would be able to use the awesome feature you have built. And now everyone is sad ☹️.

Let us try to improve this process, be quick and more effective.

1. Prepare

Let’s try not to surprise the reviewer. Discuss the approach before we jump to code. This would help in understanding the architecture and design required to implement the feature. The outcome of the discussion has to be documented in any form your team is comfortable with. It could be any form of taking documents, but it has to be traceable with PR.

Documents could be like Technical Design Documents(TDDs), Technical Review Documents(TRDs), Product Requirement Documents(PRDs), UI/UX design or just a doc. You don’t need everything, but more the merrier.

This step is very important after this everyone should be on the same page. There could be small changes later, but that should be minimal. Over time number of such changes should reduce as we are able to it plan better.

Try to split the PR into logically smaller pieces such that the entire PR still makes sense at the same time it is not too big. We need find to balance between small and complete PRs. Try to avoid huge PRs.

2. Code

Once you have a mental model of how you would solve your problem. You can start writing your code.

While writing code, make sure you add enough comments and tests.

3. Review

I generally categorize any review comment into these four categories viz. Cosmetics, Opinions, Critical, and Appreciations.

Avoid reviewing the PR along with the author. Also if possible we should checkout the branch in local and run the feature. This will give you clear idea about the feature

Cosmetics

These are changes like tabs vs spaces, single quotes vs double-quotes. Anything that doesn’t affect the quality of the code, but mostly for “consistency”.

We would want to avoid such comments in the PR. Ideally, we should have some tools like formatter(eg. prettier for Javascript) and linter(eg. eslint). So that there is no human intervention required. We would have to agree on these rules, once we do that there shouldn’t be any arguments.

If you are not able to find automated tools for the guidelines you wish to follow, at least document the guidelines. And the team can just follow the guidelines. I would prefer to avoid such things. It would look something like Airbnb’s JS guideline, this guideline could be dogmatic. Make sure to update or remove a guideline if it is not helping your team.

Opinions

We would want to reduce these too. These are generally such comments where there are x,y and z ways to solve the same problem. Our preparation step would help us a lot in reducing such opinions during the review like different abstractions or handling “future” cases, etc. We would want to avoid such comments at this stage as much as possible.

Critical

This is the real deal. You might want to comment out the typos, possible bugs, pitfalls, etc. If you are not able to understand why something is done in a certain way, feel free to ask the author to document their reasoning. Try to avoid offline communication here (like the author explaining to you on call why that change was done). You would want those gaps in understanding to be documented.

Appreciations

Something we might miss during these reviews is appreciating the author. If you find something cool, or some smart way to solve a problem, long overdue refactoring, or even first PR of a new member in your team🥳. These appreciations are really important.

Make sure you do appreciate 😀.

TL;DR

• Discuss the approach before starting to code, document it.
• Have tools to format and lint code, avoid those comments in PR.
• Comment important things, ask for documentation for gaps.
• Appreciate the author

Be kind.