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...
For further actions, you may consider blocking this person and/or reporting abuse
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.
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.
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.
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) 😅
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 😂
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 😁
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).
We are using prefixes for close to every comment:
Minor:means the commitees can chose whether to improve that oneMajor:this one needs to be adressedQuestion:...works quite well for us
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
So,
Thanks much! And I appreciate your efforts for adding more knowledge about writing code reviews. Cheers!
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 💃
Haha. Okay. COOL. B)
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.
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!
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.
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.
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. ✨
"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.
Yesss!!! I am a big supporter of the nitpick idea. Makes people accept me for being picky hahaha
haha that is true and you don't have to apologize by saying
sorry for being pickyGreat article Ankita!
I have couple of comments 🐿
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
isNaNfunction 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.
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 ;-)
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.
Loved this post!
Glad you liked it :)
Very clear, useful and a real post made for humans. Thanks Ankita
Glad you liked it 🙌