loading...
Doctolib Engineering

How to make great code reviews? 🀝

segolenealquier profile image SΓ©golΓ¨ne Alquier Updated on ・3 min read

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! πŸ™Œ

Posted on May 28 by:

segolenealquier profile

Ségolène Alquier

@segolenealquier

Software Engineer @Doctolib πŸ₯ Sometimes teaching, mainly playing with React, Rails & Node πŸ‘©πŸ»β€πŸ’»

Doctolib Engineering

We are hiring in all positions. If you want to build a better healthcare system, visit our career page: https://about.doctolib.fr/jobs

Discussion

markdown guide
 

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.

 

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 πŸ™