How to Handle Pull Requests Without Making Enemies

It doesn’t matter what technology is your specialty – if you work at a software company, chances are you are facing pull requests on a daily basis.

There are multiple benefits of getting your code reviewed. Reviews help improve the quality of the code, they are a good way to share knowledge between colleagues, and you are always in the loop about what’s going on with the project. As the saying goes, two heads are better than one.

However, when you have to review a pull request with more than 99 files containing over a thousand lines of code changes with absolutely no context, description, or links to tasks, that’s a problem. And to top it all off, such a request was opened at 16:59 on a Friday so it is the first thing you see when you come to work on Monday.

Fortunately, that situation can easily be avoided because anyone can improve their pull request description writing skills by following a couple of simple, but important steps.

According to Github, “Pull requests let you tell others about changes you’ve pushed to a branch in a repository on GitHub. Once a pull request is opened, you can discuss and review the potential changes with collaborators and add follow-up commits before your changes are merged into the base branch.”

In other words, the purpose of pull requests is to show your newly implemented code to your colleagues to get feedback. Contrary to popular belief, when creating a pull request, your main focus is not the code itself. You need to think about the reviewer, make it simple for them to process your code, and make sure they understand what you did and why you did it that way.

When a reviewer opens a PR that he is invited to, they get an unordered list of changes that had been done. This is not such a big problem with smaller PR’s but if there are multiple changes included in one PR, things tend to get messy quickly.

If that is the case, the reviewer can rely on another important thing – commits. Commits should be available in an ordered list, sorted by the time when they were created. Looking at changes commit-by-commit can be helpful, especially if they are accompanied by useful messages.

Still, there are some problems with this approach. The reviewer is missing the big picture of what had been done in a PR. A large number of commits can take a while to get through and most likely, the reviewer will be looking at code that’s already become obsolete. If you used so many commits, it usually means that you implemented something in, let’s say, commit number 13, and then completely overrode those changes in commit number 20.

To help our fellow colleagues, it’s a good idea to accompany pull requests with good descriptions. Technically, we’re allowed to write anything in a pull request message. However, describing in detail why our code does what it does can give the reviewer a much better understanding of the problem it solves. Any piece of relevant information that could be helpful to the reviewer should be included, such as links to documentation or design files.

When compiling a description, try to put yourself in the reviewer’s position and consider whether you would have a hard time understanding the code with the information provided.

If you find it too difficult to write a good description or you are already two pages in and still have plenty to say, it might be a good idea to break your PR into multiple small ones.

The reasoning behind this approach is simple. It’s much easier to review smaller pull requests than go through hundreds of file changes in a single one. The larger the pull request, the higher is the chance of missing a bug or an edge case. Additionally, smaller pull requests get reviewed faster and are easier to merge since there tend to be less conflicts.

When you’re adding a large portion of new code, it is much better to make multiple, logically organized pull requests. Your VCS provider can be of help here because it probably enables you to stack your PRs on top of each other and then handles their re-targeting when the base gets merged.

Keep the person who is going to be reviewing the PR in mind. You are making their life easier when your description is formed in a way that brings them up to speed with minimum information.

If the reviewer is familiar with the codebase, it will generally be much easier for them to check your code changes and their interactions with other code.

You need to define what problem you are trying to solve, usually in the form of a ticket.

You’ve probably set up your VCS provider (e.g. Github) to link your tickets to their respective PR’s automatically. If not, I highly recommend you set it up that way using official Github information. In case you don’t use that setup and have absolutely no desire to start, a link to the task/ticket should also be enough.

Generally speaking, complex problems require more detailed explanations. However, you should bear in mind that complexity is not necessarily correlated with the number of line changes that you performed. For example, one of the longest pull request descriptions I wrote was for a one-liner that caused a subtle memory leak. Without a detailed description, the reviewer would have a hard time seeing why that particular change fixed the issue.

When something requires a detailed description, it’s also a good indication that the code itself would probably benefit from some comments. If your solution was hard to explain to the reviewer, other developers working on the project might have a hard time understanding it, too.

When you’ve described the problem, you’ll need to explain your solution, too. Help your reviewer by providing answers to three questions about your implementation – what, how and why.

First, tell them what you changed. Use two tabs in your browser, one for writing the description and the other to check the files. Go through the changes one by one and write down one sentence for each of them. This way, you explain your actions to the reviewer and address any unusual or confusing parts of your PR upfront.

Since you’re the one who has opened the PR, you probably know best what changes will be most confusing for the reviewer.

The next thing that you’ll want to write down is how you did it. For example, you can state that you took the implementation from another project, removed unnecessary code, replaced a library, etc.

Finally, you help the reviewer by telling them why you did it that way. Perhaps you wanted to try out a new approach, needed to improve performance, or something similar.

Optionally, you can create a checklist or a template for yourself (or your team) of what you need to do before opening a PR. For example, include the ticket, include all changes, provide screenshots/videos, etc.

If you find it too difficult to explain something in the description, feel free to start writing comments that point directly to individual lines of code. You can then group related changes together.

If the UI was changed, provide screenshots. Even better, provide the “before” and “after” screenshots. Videos are usually really useful when you need to show an app’s flow. Again, try to use “before” and “after” shots.

We’ve covered writing pull request descriptions like a pro, but what happens if you’re on the receiving end? The reviewing process can be just as complicated as opening a high-quality pull request, if not more.

Luckily, there are some tips and tricks that can speed up and improve your reviewing skills.

When I’m invited to write a review on a PR, I find the approach from this article helpful. Here’s an example:

Don’t write: This is not worded correctly.

Do write: Suggestion (non-blocking): This is not worded correctly. Can we change this to match the wording on the marketing page?

This way the author of the PR knows that this is in fact only a non-blocking suggestion, while the reviewer is already providing a possible solution. Another example:

Don’t write: Issue (blocking): According to the design, these buttons should be red. Please change it.

Do write: Suggestion (security, blocking): Let’s try not to use SHA-1 since it is not recommended anymore. Try to use its cryptographically stronger counterpart (SHA-2 or SHA-3).

With this approach, you and your team you can define multiple labels to use and improve your code-reviewing experience.

Try to avoid saying change this, do that, improve this, etc. and use phrases like Have you tried this in order to achieve better results?, Let me know what you think, Have you thought about this? It may better solve your problem in my opinion.

Try to avoid the words should and must.

Finally, if the PR is too big and you find it too difficult to review, feel free to request a more detailed explanation, more screenshots, or in the worst-case scenario, request the PR to be broken down into smaller pieces.

If a reviewer left some comments that require a response, it is your responsibility to answer them, and do it as soon as possible. This shows respect for your reviewer’s time because they will be able to continue reviewing the code without lengthy pauses.

Merging your code into an existing codebase is a two-step process. First, you need to write your code so that it is clean and correct. The second part is presenting what you did to the reviewer in a simple and understandable way, which saves both your and your reviewer’s time.

Each pull request requires a specific approach. The person opening the PR has the most information to put together a quality pull request description. They know the context, what was done and why, who is the reviewer, and any additional information that might prove useful.

Instead of blindly following the instructions above, just use common sense. You probably don’t need three pages of explanations and five screenshots if you’re just removing a harmless comment or slightly reformatting the code. Don’t try to force yourself to use each and every step described here, but instead only use the parts you believe will reduce the burden on the reviewer.

Hopefully, these tips and tricks will help you write better pull requests, which makes the whole code reviewing process quicker, easier and ultimately leads to better results.

This article was co-authored by Anteo Ivankov.