Today at @doctolib we had a very interesting workshop about code reviews: why they matter, how to ensure they are done well and how they serve both the team and the product quality.
I thought it would be interesting to share what I learned! 🤓
What is a code review?
Once you've finished working on a feature, you can push your work on your branch and make a pull request. Another developer will then look over your code and consider if your work can be merged to master (or another branch you specify).
What are the goals of a code review?
- ✨ Improve code quality and coding standards
- 🕷 Find bugs as early as possible: the sooner, the better
- 🎓 Share knowledge and learn from others: regardless of your level of seniority, you can learn a lot!
An example of a good code review workflow
1 - 🛎 Know there is a pull request waiting for you
- Set notifications on GitHub / GitLab
- Check your project management board (Jira, Trello, Asana...)
💻 As PR creator :
- Tag reviewers on Github to make sure they are notified
- Or ping them directly on slack or whatever tool you use
2 - ⏳ Take time: schedule the code review
- Set a dedicated time to focus when you are assigned a PR to review
- Or you can even have recurring dedicated times in your calendar
💻 As PR creator :
- Don't hesitate to suggest a slot to the reviewer so you can go through the code together
3 - 💡 Get to know the task
- Check the user story or bug report: to fully understand the context and the expected behavior of a feature or fix
- Ask someone who knows: the developer who coded it or the product owner in charge
- Read the tests: they can also be a good way to understand the expected behavior
💻 As PR creator :
- Don't hesitate to add screenshots / gifs of the feature so that it will be easier for the reviewer to understand what's going on
4 - 🕷 Find bugs: what to look for?
- Make sure the whole story is covered: check if the tests and implementation cover the edge cases you can think of
- Performance issues: look at performance to make sure there is nothing slowing down
- Conventions: make sure the naming, syntax, etc... respect the standards defined
- Code quality/clarity: check if the code is easily readable and understandable and whether it could be refactored
- UI design: make sure the changes respect the wireframes and user experience
- Security: verify you don't see a security breach introduced by the PR
💻 As PR creator :
- It's your responsibility to test as much as you can and ask for the PR only when it seems in order to you
5 - 🤝 Give feedback
- It's harder to communicate in writing: remember that text can easily be misinterpreted so don't hesitate to emphasize what's good and choose your words wisely
- Ask open questions: to make sure you understand the code and decisions
- Have empathy: make sure you're making suggestions in a positive way and there is no frustration if things need to be modified
- Focus first on what matters the most: don't spend too much time discussing minor syntax errors if you also spot serious bugs or security breaches
- Request changes if necessary
💻 As PR creator :
- Remember it's your code that is criticized... not you!
6 - 🎉 Approve the pull request!
When should you approve a pull request?
- when the feature does what it is supposed to do
- when you feel comfortable maintaining the code
- when you feel comfortable debugging the code
This great workshop was made by Emmanuel Gautier, thanks! 👏
If you have other good practices, tips or tools for code reviews, I'd love to hear about them! 🙌
Top comments (7)
Fantastic post!! Very straight forward and to the point. I am so glad you mentioned about "Get to know the task", reviewers often don't do this and go into the code with zero context as to why the changes have been made. And I do push for task links to be in PR descriptions to emphasize this. I also really like the notes for the PR creator, it can be easy to forget that you also have a responsibility to the code review, as the creator.
Thank you for that comment! So glad you found the article helpful 🙌.
A good coding review definitely relies on both parties...
I'm curious, do you have a sort of PR description template for you and your team? 🤓
I don't neccassarily have one for the team, but I follow one that I found a while ago (it might even be from another dev.to post):
Summarize the change in less than 50 characters.
Because:
Explain what was done in this commit with more depth than the 50 character subject line. Remember to at 72 characters
Include additional notes, relevant links, or co-authors/reviewers
I love this post, very straightforward!
Thanks 🙏. Glad you liked it!
Highly practical, thank you 😄
Glad you liked it & found it useful! Thanks for saying 🙏