How do you review Pull Requests?

Same as @ohm, on GitHub, but I want to go beyond the technical answer.

When I review PRs I’m very deliberate about my choice of words. I never write “you should do X” but I opt for wordings along “from where I’m standing it might be better to do X because of reason Y”.

I always explain the reasoning behind my suggestions and make liberal use of GitHub’s “suggestion” feature to ease integrating my suggestions for the author.

I’ve actually did a lightning talk on the topic a few years back, you can see it here (here is a great article on the topic which I also reference in the talk).

When do you review Pull Requests?

Usually in the morning or directly after lunch. I try to avoid interrupting my focus to do code reviews so I do them “when I have time”.

How long are you willing to wait for a review / approval?

As long as necessary. Usually they’re done on the same day, sometimes I need to wait another day. If it takes longer than that I tend to re-request the review as a more gentle version of pinging people on Slack.

How long do you spend reviewing a Pull Request?

Anything between 5-10 minutes for short PRs to up to an hour for bigger/complex PRs.

I really go through the code and try to understand the flow of the program which takes it’s time but also lets me spot things a more superficial code review would miss.

How many Pull Requests would you review in a day?

On busy days I’m reviewing around 3-4, haven’t had to review more than that so I can’t comment on what my “upper limit” would be.

What is the best size (number of modifications/ additions) for a Pull Request?

It’s great if the PR can stick below 100 changed lines that’s great because it eases the burden to understand the change. Sometimes that’s not possible but I think it’s a good rule of thumb to try to keep PRs small.

I struggle with this myself from time to time.

How frequently do you submit Pull Requests? (daily / weekly / monthly)?

A few times per week. Not necessarily daily.

How do you feel emotionally when someone suggests changes?

Depends on how the suggestions are phrased.

See my section above on how I do code reviews.

How do you feel emotionally when you suggest changes?

As said before, I always try to stay compassionate when reviewing and try to put myself into the author’s shoes. From the feedback I’m getting from colleagues I think I’m doing quite a good job at this.

I would try not to take anything personally (so long as the feedback wasn’t rude). Just keep in mind that often personal taste, direction and experience are factors, and if a change is suggested it just means they are looking for something different, which may be based on their taste or experience.

For people suggesting changes I would say please just be kind

• How do you review Pull Requests?
  • On GitHub
• When do you review Pull Requests?
  • When I have time after I get assigned as reviewer or pinged to review
• How long are you willing to wait for a review / approval?
  • As long as it takes. We do not merge unreviewed or unapproved PRs, even for production critical code.
• How long do you spend reviewing a Pull Request?
  • Depends on the PR, but generally not more than 15 minutes. If the PR is too big, we tend to mark our review as “Too much code, please split into smaller more reviewable chunks”.
• How many Pull Requests would you review in a day?
  • Between 0 and 10. It depends on the day and what project the other developers are working on.
• What is the best size (number of modifications/ additions) for a Pull Request?
  • I don’t think there is a “best size” or a “one size fits all” answer to this. If you’ve made the same change in a thousand files, I’ll still count it as one change. Make the changes relevant to the problem you’re trying to solve.
• How frequently do you submit Pull Requests? (daily / weekly / monthly)?
  • Again, it depends on what I’m working on. I would say around 5-10 per week on an average week.
• How do you feel emotionally when someone suggests changes?
  • I feel happy, because it means that they care enough to actually check out what I’ve been doing. If I agree, I’ll give a and immediately fix it. If I don’t, we can have a discussion. Sometimes that discussion happens off of GitHub (Slack, Zoom, in person) or it can happen in the review. Once we come to an agreement, we’ll post the agreed upon changes and I’ll do that.
• How do you feel emotionally when you suggest changes?
  • If the change is “you misspelled this” or “Please follow the style guide” I get annoyed, because it feel like the other person just don’t care. When I suggest changes other than that I tend to back it up with “previously we’ve done this in file X on line Y” or “We discussed here LINK that we would do it this way” or even “I would have done it like THIS, but if you feel like we should do it your way going forward, we can do that”.