Pull Requests are a powerful tool for code review and knowledge sharing.

And, as developers, reviewing Pull Requests represent a big amount of time every week.

We will talk about "Pull Requests" in this article. But you can read it as "Merge Request" or whatever term you are used to. These ideas are not directly linked to any tooling.

But we might not want to do Pull Request

Some really knowledgeable people advise against doing Pull Requests inside a team. So, here are some reasons why we might not want to do Pull Requests:

• They introduce a blocking step in the dev process. And this step might take time.
• We do not often catch problems during these reviews.
• And when we find something to say, the feedback comes really late, and it might be too late to make changes.

We can do pair or mob programming to keep the benefits of Pull Requests without their drawbacks. Pair or mob programming put the discussion and feedback during the development of a feature instead of when it is already done. This makes for a better design and implementation on the first time.

By doing so, there should be nothing to say once the feature is done. And the team can go to another feature directly because there is no longer the need for this blocking Pull Request step in the process.

But leaving Pull Requests or doing pair programming might not be an option. So we better know how to do effective Pull Requests. And here are thoughts to try to make them easier.

Pull Requests as a team

Pull requests do not have much sense outside a team because nobody would be able to review our changes. So it starts by asking ourselves why we use this tool inside our team.

Define what you expect from Pull Request

It is not enough to create a branch, doing multiple things on it, create a Pull Request and wait while spamming our teammates from time to time so they don't forget to review it.

Well. In fact, it can be enough. And it is sometimes what is done in teams. But it is definitively not the best experience for anyone.

Instead, everybody on the team should be aligned on how to handle the Pull Requests. This means answering questions like:

• When should Pull Requests be used? Are they required for any new development? Or are they only for new features? What about documentation, styles or build changes?

• Are they used for everyone or only for contributions from people outside the team?

• What are their goal? Is it to share knowledge between team members? Is it just a tool for code review? Is it okay to suggest big changes or start discussions on a Pull Request?

• Are Pull Request a way to transfer the ownership from the author to the whole team? Meaning that, once merged, every member of the team is as responsible for the code as the original author.

• When are they reviewed? Each day? Each Friday? Each March 2nd?

• Who can validate a Pull Request? Can every developer give his approval or only senior developers?

• And how the reviewers know that they should review the Pull Request? Are they mentioned on it? Do they watch for a given label? Does the Pull Request link need to be sent in a channel in team's chat?

• When can we consider a Pull Request approved? Does it need to be approved by only one person? Or do multiple developers need to approve it before merging.

• And how to know that someone approved the Pull Request? Does the person approve it by using an "Approve" feature of the tool? By adding an emoji? (If so, which emoji?) Or by leaving a comment that says "all good for me"?

• Who resolve the comments on the Pull Request? Is it the author of the Pull Request or the author of the comment? Or is it the author of the comment if he has not yet approved it and the author of the Pull Request otherwise?

Making sure that everybody in the team is aligned with why we do Pull Requests is important. Otherwise, it could be the cause of conflicts between team members that have different expectations.

Automate everything you can

A review by a peer takes time and is often quite exhausting for the reviewer.

To make it easier on the reviewers, everything that can be automated should be automated.

This includes, if possible:

• Running the tests automatically, so the reviewers don't have to do it manually.
• Running code formatters to have consistent formatting everywhere and not letting the formatting be a distraction.
• Running linters and code quality tools for common problems and team conventions to avoid manual checking.

Automating everything that can be lets the reviewers focus on what is important. They are free from checking every whitespace and variable declaration and can focus on the big picture.

Another benefit is that the automation gives feedback earlier. The earlier the feedback, the better. Ideally we should even be able to run these automated checks locally to have the feedback during the development.

And when automated, there are way less comments to handle. The fewer comments there are, the easier it is for everyone. The author doesn't have to handle them. And the reviewers don't have to come and check later to see if they are resolved.

Find what works for you

There is no single solution that work for every team. Pull Requests are easier when everyone is aligned on why we do Pull Requests and there is automated feedback. But we still need to review the Pull Requests.

To make the review process easier, the team may choose to use its own conventions. Here are some ideas:

• Everybody can add an emoji while reviewing to communicate that he's still doing the review. This can help avoid getting comments after the Pull Request is merged. And it helps to avoid spamming someone for a review if he is already reviewing the Pull Request.
• A tool that can be useful is the use of conventional comments. This helps to give context about the comment, its intent and what is expected. It can make it easier for everyone to know what is needed. (More on that later.)

Let's now move on into the life of a Pull Request. And its starts by the author creating the Pull Request for review.

As the author

As the author of the Pull Request, we are proposing changes to our team. It is often expected that we will make the changes necessary until the Pull Request is merged.

Remember that it is the code that is reviewed

Comments on the Pull Request are comments about the code we produced and the solution we thought of. It is easy to take them personally.

But we shouldn't. One of the most important thing we can do is to remember that the comments are about the code and not about ourselves. We should put our ego aside to discuss the choices we made and search for the best solution.

It's possible that we made bad choices. There are a lot of good reasons to be wrong, including not being aware of some information, or just making mistakes. Or our solution can be a good solution but not the best possible. We should try our best to stay open to find better solutions instead of defending what we did first.

This does not mean that the reviewers are always right. Sometimes we already thought about or tried what is suggested and rejected the solution. We should explain why we chose the solution if it's relevant. We just shouldn't hold to a bad solution just because of our ego.

Break changes down

Big Pull Requests are exhausting both for us and for the reviewers. And the reviewers will not be able to do a good review if the Pull Request is too long.

To make Pull Requests easier, we should try to make them as small as possible. This means splitting big tasks in smaller tasks and doing one Pull Request for each.

If the task is big and multiple Pull Requests are necessary, it may take time to continue. To address this problem, we can stack Pull Requests. (The first points to the branch we want to merge on. The second point on the branch of the first. And we will merge the first before merging the second.)

Smaller Pull Requests make it possible for the reviewers to focus on what changed, leading to better feedback. And since they are faster to review and result in less comments, they are usually merged faster. And, since smaller Pull Requests are less of a chore, this can help team members to review them more frequently.

Explain what the change is intended to do

To make it easier for reviewers, we should explain what the changes are intended to do. It can be as easy as giving the context that made the change necessary.

Since we try to make small Pull Request, the changes should only focus on one behavior change. That makes them easier to explain.

The reviewers were most likely working on other subjects. Giving them the context of the change is a gift we give to them. This information makes the switch of context easier for them.

If the change is linked to an issue, it is interesting to put a link to that issue in the description. It gives an easy way for reviewers to get more context if they need. (For what it's worth, you might also want to put it in the commit. It can help a lot to find why something was done when debugging in the future.)

And we can add comments on our Pull Request if there are some changes where we think more context is needed. (In this case it makes sense to ask ourselves if the best place is a comment on the Pull Request or a comment in the code.)

Be your first reviewer

Reviewing our Pull Request ourselves can help catch errors, forgotten TODOs or bad structure. And finding these ourselves saves times for everyone by avoiding back and forth comments.

Reviewing our code is easier that reviewing a Pull Request from someone else because we already have the context and the big picture view of what is done. There is no good reason not to do it. (Except that we might have already done it before typing "git commit" two minutes ago.)

And it is an opportunity to add comments to explain a choice we made for the reviewers. Or even to ask for thoughts about a piece of code we are unsure of.

Then we just have to wait for the reviewers to review our newly created Pull Request and answer them or apply their suggestions.

As a reviewer

This is the hardest role in the Pull Request process. It requires us to switch context and understand what someone else did. But it is the whole point of the Pull Requests. They would serve no purpose without reviewers.

Remember that it is the code that is reviewed

The authors should remember that it is their code that is reviewed and that the comments are not about themselves. But for that to work, we have to ensure that the comments are about the code and not the authors when writing them.

We should always be discussing solutions and how to make the code better. And there should never have comments on the author. If we find ourselves using "you" in a comment, we should take time to think again if this is really what we are wanting to write. Pull Requests are usually not fun. Let's try to not make them harder than necessary for anyone.

If you see something that seem strange, it is better to ask for details about the choice. And more often than not we should not suggest that something is wrong without all the context necessary. There are chances that the authors thought about it and made a decision based on information we do not have. So prefer asking for details instead of pointing something as wrong.

Share your thoughts

Pull Requests are a communication tool. This is not an evaluation of the code someone else produced. We can do more than just saying what could be better.

It can be asking questions to better understand a choice or why the change was necessary. After all, once the Pull Request is merged, we will likely have to maintain this code. It is a good moment to go get information to understand it.

It can also be thoughts of potential issues we think of when reviewing the code. Maybe the changes may have impacts that the authors did not think about that would be interesting to check. If so, tell them. It will be easier to check it and make changes when the Pull Request is still being reviewed.

And it can even be unrelated thoughts. For example, sharing a pattern that we think might be appropriate but do not make the code a lot better. It can help to gather opinions or start a discussion to finish elsewhere later.

Be clear about what you expect

We put comments on Pull Requests for different reasons. We should be clear about the intent behind every comment so the author and other reviewers know what we are expecting.

The context of a comment includes: if it is blocking the Pull Request, whether it is a question, a thought or an issue, etc.

Conventional comments are a great tool for this. It consists of prefixing the comment with context. For example:

• prefixing with "question:" if we are asking for more details
• prefixing with "thought:" if we are sharing a thought and not making a suggestion
• prefixing with "suggestion:" if we are suggesting changes
• prefixing with "nitpick:" if what is done works, but we found something to nitpick about
• adding "(blocking)" or "(non-blocking)" to add context about if it has to be taken into account if the author wants our approval

See the Conventional Comments' website for more details. And, even if they thought of a lot of cases, we can add our own keywords if we need to. Or even take inspiration from it and do it our way with different keywords. Or with emojis.

Let the author know of what is done well

Last, it is nice to the authors know if we find something that is well done.

It can be hard to receive only feedback on what to change. Getting feedback on what is good can help the authors feel better about the review and his skills. Especially if the author is new in the team, a junior or lacks confidence.

The review is not a punishment, we should try to make it easier for everyone.

Conclusion

There are tips that I think can help make Pull Requests easier for everyone in a team.

These are my current opinions about Pull Requests. It may change in the future, it surely did since I started doing them. And I will try to keep this article updated if I change my mind.

I hope it can help you to make better Pull Requests. And, if you think of things that are missing, please reach out. I would be grateful for the feedback and would love to try new ideas.