Code Reviews
Lessons I Learned as a Code Reviewer
We are human, and we can make mistakes
We, as a programmer, are continuously building new features and shipping them to our production system to facilitate our customers.
But we are human, and we can make mistakes. As we develop new features, our implementation needs reviews by our peers.
Over time, I learned a few lessons as a code reviewer, and I am happy to share them with you.
Review your code first before asking others
As soon as you open a PR, you are proposing a solution to a given problem and discussing your solution with your peers. So, it is essential to review it yourself first to avoid typos, remove print statements, etc.
This way, reviewers will not get distracted by these minor mistakes, leading to less back-and-forth.
No “YOU” attitude while requesting changes in pull requests
The “YOU” attitude could sound impolite. For instance,
why haven’t you close the connection?
can be rephrased to:
As per my understanding, this code is not closing the connection 🤔
Adding emojis helps and reflects somewhat politeness.
Ask questions, don’t dictate
Raising a question reflects that it’s just an opinion, and you may disagree with me. You could justify it by answering my question.
Do you think it would be a good idea to surround this code block with try-catch?
Use present tense while writing commit messages
I recommend using the present tense for your commit messages, as it explains what the code is doing. I have checked public repos from AWS and Google, and they follow the same practice.
Writing Effective Commit Messages
Effective commit messages help others understand your changes. Use this template.
TICKET-ID Your message goes here
Reason - Why are you making this change?Example
NOX-123 Migrate CI/CD pipeline to GitHub Actions
Moving away from Jenkins to GitHub Actions to reduce
maintenance overhead. Jenkins is self-managed and
we're trying to get rid of this component.Adding reasoning provide better context for others (and your future self). Maybe the reason when the change was added was only relevant in the past and does not make sense now.
Keep PRs small, but provide context
It’s crucial to find a good balance between the number of commits and the context they are sharing.
Keeping commits too few without conveying the context of why we are doing this will not add value. Similarly, a high number of commits could be overwhelming and prone to overlook bugs.
Be honest and say it louder & clearly
Before reviewing any pull request (PR), I skip and skim. It gives a first impression of the implementation and how much time I need to invest in this PR.
If it makes it harder for me to understand the implementation, and if I lose focus, I say it louder and clearly by adding a general remark.
Please note that I am not impolite here.
Sorry, I am not able to understand this implementation. It’s harder for me to follow.
It is also feedback. If it's harder to understand for any members of a team, one should re-structure code or organize it into smaller commits.
Conclusion
Code reviews are crucial, in my opinion. While reviewing the code, I have the mindset that I might have to work on this feature tomorrow. If I cannot understand it now, I will not be able to understand it tomorrow.
Thanks for reading.
If you have any questions, please feel free to ask them in the comment section. I would be happy to answer them.
If you like this post, you might want to check my other articles on code reviews.
Want to connect?
LinkedIn | TwitterSubscribe to get my work directly into your inbox.
https://medium.com/subscribe/@anasanjaria🚀 Want a free 1:1 session?
Share your public project with me, and we can discuss your commit history and ways to improve it. Book a free appointment.
