DEV Community

The Art of Humanizing Pull Requests (PR’s)

Ankita Kulkarni on June 19, 2018

What are PR’s, how to effectively create a PR, how to give feedback on PR’s and how to respond to feedback What is a Pull Reque...
Collapse
 
thecodetrane profile image
Michael Cain

I make it a point to only ask questions on PRs. Even if you know you’re right, when you couch it as a question, you offer the author the chance to discover the knowledge on their own (and that way they can own the solution) rather than telling them what to do (and they rent it).

As the old sales adage goes: telling ain’t selling. Guiding people to the right answer with questions produces much better results.

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni

Wow, I wish I knew about the old sales adage before I published this post. This is awesome. Thanks for sharing this.

I also love to do the same. Always ask! I think of 5 other things when a teammate raises a question.

Collapse
 
yellowbrickcode profile image
Sarah Williams

Love the nitpick idea. I always clarify with something like “not a big deal but” so I’m definitely going to start prefixing with nit instead of doing that.

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni • Edited

Yeah, that's awesome that you loved it. nitpicks are minor but technically relevant. I love them. It removes the awkwardness of being picky (sometimes) 😅

Collapse
 
jvarness profile image
Jake Varness

Someone wrote "nit" in a code review at my company once, and someone who wasn't familiar with that term thought that was the preferred name of the author 😂

Thread Thread
 
kulkarniankita9 profile image
Ankita Kulkarni • Edited

Haha this is hilarious. I was going to suggest that we should share some common practices with new devs or new clients although we wouldn't get to giggle on this one then 😁

Collapse
 
rysilva profile image
Ryan Silva

Only thing is I've had people say "nit" to me on things that were pretty subjective (like it deviated from their preferred style but was in no way a standard for our team). In those cases using "nit" almost seems insulting, like they've assumed that their suggestion is correct. I think "nit" is fine when it's something obvious (like extra whitespace or a typo).

Collapse
 
karfau profile image
Christian Bewernitz

We are using prefixes for close to every comment:

Minor: means the commitees can chose whether to improve that one
Major: this one needs to be adressed
Question:...

works quite well for us

Collapse
 
joshichinmay profile image
Chinmay Joshi • Edited

Wow. I loved the blog. Especially where you explained, "How to review Pull Requests?". What an explanation. You have covered almost everything. Right from the documenting the pull request to merging.

Also, I am willing to discuss/add a few more points about creating a pull request. Did you forget to talk more about the points following image has? Or you don't want this to be part of the blog? I don't know. :D

pull request parameters

So,

  1. Reviewers - Reviewers are those people who are going to review your pull request. You can request more than one review to the pull request.
  2. Assignee - Assignee is someone who is currently working on the pull request. There can be more than one assignee.
  3. Labels - Labels are the best way to add identification to the pull request. Github has already given important labels. Such as - bug, enhancement, feature, wontfix, help wanted, etc. So that other collaborators can identify the pull request without even opening.
  4. Projects - This is where you can tag a project which you have already created.
  5. Milestones - By adding milestones you can track and filter your pull requests. You can find a detailed description over here.

Thanks much! And I appreciate your efforts for adding more knowledge about writing code reviews. Cheers!

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni • Edited

Hey glad you liked it. Haha yeah I intentionally left that bit out as the post was already getting too long, also GitHub has so many cool features for PR's that one post won't fit everything. Although I'm glad you added it as most readers would go through comments.

Also, there is so much more I want to share, probably a part 2 article should be written. Hmmm time for more weekend work 💃

Collapse
 
joshichinmay profile image
Chinmay Joshi

Haha. Okay. COOL. B)

Collapse
 
alahmadiq8 profile image
Mohammad Alahmadi

Thank you so much. That was so much fun to read. It is easy to misinterpret the tone in PRs. Humanizing and being empathetic to our reviewers is an important still.

Im definitly gonna start using gitmoji moving forward.

Collapse
 
jaymeedwards profile image
Jayme Edwards 🍃💻

Great article.

Agree completely with the need to be empathetic as a reviewer.

Fear and power struggles are a chronic problem in our industry during code reviews.

This alone can do a lot to reduce it!

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni

Yeah I have been through both. Honestly we all want to feel inclusive right? So if we do our bit, we would get to write better code collectively.

Collapse
 
conradbeach profile image
Conrad Beach

Another great tool for creating GIFs is Kap. I used LICEcap for a while before trying Kap. They're similar in a lot of ways, but Kap has, in my opinion, a nicer interface with some additional helpful features.

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni

Yeah, that's a great tool. Thanks for the recommendation ✨. I will start using it now and you are right, Kap has a much better interface than LiceCap. ✨

Collapse
 
weswedding profile image
Weston Wedding • Edited

"Take it offline" can't be emphasized enough, imho. Expand that advice to client communication, or cross-team communication, etc. Learning to sense when it's time for a call is a really good skill to have.

An imperfect signal that it's time to walk over for a face-to-face or give a call is if someone's kind of stressing out and the email you're writing to try to assuage a concern is hitting 2-4 paragraphs in length. And you're spending 30 minutes trying to be very specific and clear in your language.

Collapse
 
milkstarz profile image
malik

Yesss!!! I am a big supporter of the nitpick idea. Makes people accept me for being picky hahaha

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni

haha that is true and you don't have to apologize by saying sorry for being picky

Collapse
 
sapegin profile image
Artem Sapegin

Great article Ankita!

I have couple of comments 🐿

this function basically checks if the value is Nan and returns a boolean.

I think it's not the best example. First, if it was really “basically”, you wouldn't need to write a comment. Second, the comment merely repeats the function name itself.

Better example would be something like: “I'm using the isNaN function from Lodash, which I've never used before, and I'm not sure it's the right way to use it”.

Also if it's something really tricky or unusual, better to write a comment in the code, not in the pull request.

If you think the feedback is valid, implement it! However big or small the change is, if you think the feedback is valid, always reply and let the reviewer know that you took their feedback.

As a reviewer I don't completely agree with this. Receiving 20 comments with “fixed” for a particular pull request isn't fun. As a reviews I expect that all comments will be addressed in some way: by changing the code or explaining why it won't be done. Saying something like “thank you, great idea” occasionally is a great thing though ;-)

Make your commit message obvious

I think this depends on a workflow and a particular team. Other all commits get squashed into one before merging, so only one commit message per pull request matters. Which should be very good ;-) Also as a reviewer I don't really care about particular commits, but maybe that's just me. For me a comment by the author, saying that the pull request is ready for another review iteration, is much more useful.

Collapse
 
ben profile image
Ben Halpern

Loved this post!

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni

Glad you liked it :)

Collapse
 
brasileiro profile image
Daniel Brasileiro

Very clear, useful and a real post made for humans. Thanks Ankita

Collapse
 
kulkarniankita9 profile image
Ankita Kulkarni

Glad you liked it 🙌