DEV Community

Cover image for Effective COMMENTS on pull requests
Tan
Tan

Posted on

Effective COMMENTS on pull requests

This post is not about how to write a good pull-request, for there is a lot of great material about that in this blog and also online. Instead, this post is about writing an effective comment on pull requests.

I'm not the first to come up with this topic. During my last internship, I attended an amazing discussion titled "Effective Code Reviews". The focus was on the reviewers themselves, rather than on the author.

Since then, I wanted to share more of what was discussed. Thus, I hope that this post helps you write better comments, but also allows us to further discuss this topic.

Introduction

So you might say:

"When I review a pull-request, I know how to approach someone with constructive feedback. I would never write:

A mean comment. Please don't ever do this.So how could I possibly be as effective as that?"

First of all, yes. Please never write something like that. Indeed, explicit toxic behaviour is easy to identify and avoid. However, sometimes we unintentionally may harm each other with a seemingly innocent suggestion.

We as humans cannot run away from our nature. We are inherently defensive. That's how our brains work. We have our biases, our preferences, our beliefs, and that's fine!

A (not) very uncommon scenario

With that said, the point of a pull-request is not to write impeccable code. It's for us (author and reviewers) to learn how to improve the current state of our codebase.
So how, in doing so, can we accidentally harm our effectiveness (and ourselves)?

Let me tell you a story:
After two weeks, you finished the most expected feature of your P.I.E. system for this iteration. You've spent hours and hours reading your code, looking for typos and trying to test everything as much as possible. It is finally time to deploy it, and you submit a PR to the production branch.

You and I work on the same team, so you add me as a reviewer to your PR. Then, you go do something else, while I review your changes before I can approve it.
You take a sip of your coffee, and can't contain a smile. You've done it. After all this time, you're allowed to rejoice.

However, thirty minutes later, you see that I've left some comments on your PR. You open the page, and see this:
Some comments that aren't exactly ill-intended.

...Let's say this goes on for more five comments or so.
How would that make you feel?

You're not necessarily offended... But you do not feel good either... right?

Instead, look at the parallel universe in which I left a different set of comments:
Somewhat more kind comments. Would you agree? How would you feel reading those instead?

Effectively reviewing a PR

Leave more questions than suggestions

Ask a lot. You should first understand what the code is doing, before even trying to suggest an improvement. This helps to achieve two things:

  • you learn more about the code
  • you could help the author spot a small bug that they didn't notice.

Ask more questions than leave suggestions.

PS: If you've discussed something in-private, remember to add an update so other people from the team who might have the same question also know what was discussed.
Leave updates if something was discussed in-private.

You can also leave compliments

I would highly encourage you to do so if you can. Pull requests aren't only for pointing out things to improve. You can also point out positive things, like:
A nice positive thing.

Another positive thing.

If it's too big of a change

We want to write the best code we can, but sometimes we have to stick with good enough. Instead of pointing out changes that couldn't be achieved in a certain timeframe, just mention it.

A change that might be to big.

Don't hide answers

If you're asking for a certain improvement, tell how it could be achieved.

Do not hide improvements. Just say it.

Avoid using "YOU" as much as possible

This is a minor one, but remember we're judging the code itself, not its author.

Avoid using

Conclusion

We as developers must contribute to a diverse environment, where different opinions are heard, and discussed respectfully.
I hope these tips help you all to do better reviews.

It's ok if you don't agree with all of these. So I would love to hear what you think in the comments. What do you all think?

This is also my first post on this blog, and I hope you've liked it!

Kind regards,
Tancredo.

Top comments (8)

Collapse
 
peaonunes profile image
Rafael Nunes • Edited

Nice post! Those are very good points.
At Airtasker we found Conventional Comments to be very useful.
The idea is that you set the expectation around the comment before the comment. So it avoids a lot of assumptions on both parts of the reviewing process.
Everyone states clearly the objective of the comment, but also its urgency.
Your examples could look like:

question: I do not quite understand how updateClientInfo() works, could you give an explanation? Do you think it worth a brief in code documentation?

suggestion (non-blocking): We've been having some trouble with this APPLE...

suggestion (non-blocking): What do you think we call this function isBufferAvailable()...

suggestion: When this function executes with pi = 3.14, it causes a stack overflow...

We do not follow it strictly, but just by using some of its concepts improved a lot the quality of our reviews and also the empathy on both sides.

Collapse
 
tan profile image
Tan

Thanks for sharing Rafael! I will definitely use this for future reference.

Collapse
 
kehoecj profile image
Clayton Kehoe

Nice post! I like the approach of asking a lot of questions and have used that in my code reviews too. One counterpoint is that on teams with high levels of trust I would rather have the first example of PR comments. For example I would much rather someone say "m is not a good name for a variable, you should name it message" rather than coming over and having a side discussion on it. If I disagree, I can comment back and then other on the team can chime in.

Collapse
 
mhsangar profile image
Mario SΓ‘nchez GarcΓ­a

Recently started working with PRs and I think this will be super useful!! Thanks for your post, Tan πŸ‘ŒπŸ»πŸ‘ŒπŸ»

Collapse
 
tan profile image
Tan

Thank you! Glad you liked it.
Hope you have a good time reviewing those.

Kind regards!

Collapse
 
adamnator92 profile image
Adamnator

Good sharing, every developer should read

Collapse
 
shyynux profile image
shyynux

This is such a helpful post, i will try my best to be an empathetic reviewer, i have seen ego games in review comments, it is so hurtful if we are put down

Collapse
 
riimako profile image
Riina Korpela

Those are very useful points! Sometimes it's just hard to keep the comments nice, and these are good tricks :)