How I like to approach PR reviews

PR reviews are such a huge central part of our daily routines that how you actually approach them can have an impact on the other developers

Actual picture of you looking at your PR before it is reviewed

Pull request review is a really popular mechanism that we have today that allows engineers to validate a set of changes that will be applied to the source code, in order to have a verification step before merging them. It is a great step to check for code legibility and maintainability, as well as validating that our code is doing what we expect it to do, and that we are taking into consideration the different variables that could affect that code.

That is as far as code is concerned, but what about the people involved in that PR 🤔? How does that review affect the developer that created the pull request in the first place? Did we ever get a training on how to do these reviews, what to look for and what is the best way to provide feedback?

Over the last couple of years, I have seen a lot of PR reviews and I have learned the things that I like about PR reviews ❤️, the things I do not like so much about them 😅, and how I like to approach them, hoping that when I create a PR, it will be treated the same way as I did. So here are some of the things that I learned from reviewing and creating PRs.

Try not to assume, ask and discuss!

I am pretty sure that you have already seen these comments on a PR before:

(these are real examples, and no, I did not write them 😅 )

There is a common factor between these types of comments, and it is that the reviewer is assuming why the author did it like that specific changement the way they did it, and that for the reviewer, it should not have been executed like that. As a consequence, the comment can be perceived not only as a harsh critic, which could potentially diminish the work that the author did to have these changes.

Let's take the "Adding this function here makes no sense" comment as an example on how we can improve it. The comment is stating that a specific function made no sense where it is, but how does the reviewer really know that it does not make sense there? Maybe it is a function that could be moved to a utils package on the application, maybe the reviewer saw some potential on reusing that function somewhere else, or maybe they just don't like where it is 😅.

But does that mean that the author did not consider those aspects while they were writing that code, so they decided to go ahead and add that function there anyway? or does it mean that the author has a larger context on the PR that they are developing and they could have perfectly created that code change as it is for a specific reason 🤔 ?

Since it is impossible to answer those questions with only the code, I like to approach these scenarios with the philosophy of ask, rather than assume and command.

So in this comment, we can see a different approach to suggesting a change on a PR. The reviewer does not assume that the PR's author did that change willingly and asks for more information in order to determine the logic behind said change. This could potentially have the following advantages if we compare it with the more "direct" comment:

• Creates an environment where both the reviewer and the author can discuss as peers the proposed change, and allows the author to add more detail to the created solution. By opening a discussion with questions rather than orders, the comment is less perceived as a critic and more as reflection on the current solution and whether it could be improved.
• This allows the author to explain their train of thought and why they made that change the way they did. Their reasoning could be 100% valid, and maybe it just needed a couple of comments or better naming in order to correctly convey it.
• Junior developers can profit from these types of exchanges, since it promotes an enviroment where the PR's author can think about the approach that they took and reflect on their decision making process. Comments like "This is not useful" tend to gatekeep these developers from since they do not have a chance to discuss the changes in a healthy manner, they will just follow that comment like it is an order. And also it sets an example on how we could potentially approach PRs.

Review your own PRs!

This is one tip that I found really, really useful while working with PRs. Once I have created the PR, I usually take my time to read it and go through the code. By doing that, I can anticipate some of the questions and comments that others may have, and by doing so, accelerate the review process and make my code a lot cleaner and more readable.

The idea here is to go through your code as if it was not yours, and review it. While we are developing, things make more sense when we code them, since we are in that sweet spot where we know every aspect of the code we are writing, how it impacts other parts of the code and how it all comes together. But reading just the changes is really another story, and gives you the perspective of what a reviewer will end up seeing!

So what I do is, I create the PR and while I wait for the CI checks to pass, I go through my code and do some validations:

• If there is something on that code that is not extremely clear to me, or if it looks like a complex logic, I will go ahead and commit some fixes in order to make more clearer. Maybe I will add some comments or change the variables names, so I can make sure that the reviewer will clearly understand what I was trying to do and not go through the same problems I had while reading my code.
• If there is something that lacks context (like a function creation that shows up on the changes and later on it is used), I will go ahead and add some comments myself! GitHub allows you to add individual comments to your own PR, which comes really handy when you want to let the reviewer know that something will be used later on, or that you still need to add tests to a certain module, etc.

This approach will not only give you the chance to look at your code from another perspective, but also learn what things you could review in another PRs with other developers!

Add some flavor to your reviews!

Last but not least, while I am doing a PR review, I want also to take a moment to celebrate the work that my team mates have done and point out the things that I really liked. PRs are usually filled with comments in order to improve or change a certain code, but we really do not add anything saying "Hey, what you did here, is awesome!". And in some way or another this feels like the natural thing to do, since it is a review 😅 Usually in reviews you say the things that you did not like about something and you also comment the things that you really liked. Why do we not take the same approach for PR reviews?

So whenever you are doing a PR review, think about the things that you liked, not only the things that you thought they could be better. If you see something cool, something that caught your eye and made you say, "hey, this is actually pretty good", just add a comment and make the PR's author know! Take advantage as well of the possibility to add a message once you have approved a PR. Take that moment to celebrate and encourage the author to keep up the good work!

And add some Gifs or emojis while you are at it!