A pull request isn't just the responsibility of the author, but also the reviewer. In this article, I'll be sharing 8 tips on giving effective code reviews.
1. Practice conventional comments
I've been practicing conventional comments for around 6 months. I find it to be a good practice. Here are a few reasons.
- The intention of our comment is clear
- The separation between the topic of the comment and the example and / or reasoning of the comment is clear
- The author knows what they should prioritise on because our comments are well-labelled
2. Suggest changes by masking them as questions
When we mask our suggestions as questions, they can be thought-provoking to the author and the tone of our suggestion is highly likely to come across as friendly. Here are a few examples.
- What if we rename this boolean variable from
invitationAcceptedtohasUserAcceptedInvitation?hasUserAcceptedInvitationis more clearer thaninvitationAccepted. - Since we're doing
${address.city}, ${address.country}more than once throughout these files, how about moving this combining of city and country into theAddressclass?
3. Add an example in our suggestion
Adding an example in our suggestion can be more beneficial than leaving just a reason. An example coupled with a reason might just give the author a eureka moment. 💡
4. Replace 'you' with 'we'
The effect of using 'we' instead of 'you' with people that know us well may not be significant, but with people we've not worked with before, it can provide a psychological difference, especially to those new to the industry. It can make them feel welcomed and makes the code review feel like a learning session.
5. Mind your nitpicks
Some nitpicks are necessary, but some of them are not. A code review filled with unnecessary nitpicks can affect the author's self-esteem and might even make us look unkind. How do we decide when to nitpick?
I suggest asking ourselves whether the code we're reviewing is misleading.
Is there a typo on user-facing content or a variable with a misleading name? If there is, it is a necessary nitpick.
Is our suggestion purely a stylistic change that makes no positive difference in readability? If it is, then it is an unnecessary nitpick.
But what if we want to enforce a convention within the codebase such as using double-quoted strings?
Then this should be taken care of by a linter. In JavaScript, there is ESLint. In Ruby, there is RuboCop.
6. If you're reviewing code, dedicate some time for it, don't rush
Reviewing code is also a part of the job. When we do our code review in haste, it's easy for sneaky bugs to slip through. If you're in a rush, I suggest doing the code review later if it's not urgent.
7. Use emojis and GIFs!
Most of the time, we communicate with the author of the pull request through text. It's easy for them to misinterpret our comments negatively, especially if we don't know each other well. Using emojis and GIFs can keep the atmosphere warm. Though please avoid leaving sarcastic emojis or GIFs because sarcasm doesn't transfer well through text. 😁
8. Leave a summary when necessary
Sometimes, we encounter some code that are similar and require similar changes. We shouldn't be leaving the same comment more than a few times as it can make the author feel overwhelmed. Instead, leave a summary. Here's an example.
The rest looks good to me! Just a few minor suggestions.
- Link to code block A
- Link to code block B
- Link to code block C
- Link to code block D
These blocks of code are doing the same thing. How about creating a reusable method?
Summary
When we review code, the objective isn't just to tidy up code and then merging the pull request for the sake of keeping a project moving. A code review is also a place for learning. Not just for the author, but also for the reviewer. Making ourselves approachable is one way of making code review a fun process.
If you find this article helpful, please spread the word! If you're looking for tips on opening effective pull requests, this article is just for you!
Do follow me on Twitter to read about my findings while programming or my feelings of joy and despair while watching football. I'm also active on Instagram and Goodreads where I frequently share what I'm reading and what I've read.
Thanks for reading! ✌️
Top comments (5)
Thanks Ali for sharing this!
I would distinguish two kinds of code reviews, which any developer could do:
1) Check code style to be it as "Clean code" suggests and that it follows project's code conventions
2) Do check code "adequateness", if it provides a proper solution and not a dirty hack.
That it's design is good enough for the project.
First type could be done by anyone.
To be more efficient with the second you should be an "expert" in a code which you do review. So, some tools could be useful, which suggest recommended reviewers.
Thanks for the feedback, German. 🙂
The first type of code review can be done by anyone, indeed. Having linters help too!
Regarding adequateness, I feel there are two ways to review this. Design discussion and code review. I try to hold design discussions if the feature / fix being worked on is major and / or extensive. Changing the approach before you write the code is more efficient than changing the approach after going through a code review.
Indeed, absolutely agree that it is better to review approach before implementing it.
Thank you Greg! May your team find these tips helpful. 🙂
Thank you Erick! 🙂