This is an anonymous post sent in by a member who does not want their name disclosed. Please be thoughtful with your responses, as these are usually tough posts to write. Email firstname.lastname@example.org if you'd like to leave an anonymous comment or if you want to ask your own anonymous question.
I just started my first job as a junior backend dev! I'm really curious to hear from DEV users as to what they look for when reviewing code for your team. I don't think I'm going to be the first person that's asked on my team, but I'd love any tips and tricks to know how best to review code and how it can be most helpful to that team member in question.
Oldest comments (11)
As usual, any detailed answers will have 'it depends' at the start... 😁
The above assumes there has already been much automation to help with syntax / code style and such like applied before human review!
Not getting personal and not getting lost in the details.
It depends largely on the guidelines you have.
If there are guidelines saying, you should always use early returns and you need good reasons for not to, you should mark it as a problem when reviewing.
But if you don't have such guidelines, many things come down to taste. If you see a potential bug or edge-case, tell people, otherwise, leave their code as it is.
I would recommend this post and it’s second part. This is really helpful.
Awesome Read, Thanks for posting it here.
Give constructive feedback, like: „Did you think of doing this way? This could be more… . Is there a specific reason why you chose x over y?“
Not just: „This is bad. We should not do it this way.“
Stay neutral in tone.
Thank you for your detailed response
Assert good intentions and that the author was trying their best :)
It is important to separate facts from personal opinion for psychological safety. Maybe we need a template for review comments for that. I haven't been able to do it.
I guess the first point is: getting used to the existing guidelines! You should be basing any PR review on those to avoid personal views and, if they do not exist, question Senior Devs about it and write them down, maybe even document those guidelines yourself (and ask for their approval when done).
Other than that it's basically looking for business logic inconsistencies (which you'll only know if you know the context of the changes) or more wide topics, such as security and performance.
The most important thing of all is to remember that it is not a competitive sport, it is a collaborative process. Be polite and kind in your collaboration. If you can't express your point politely and kindly then you have nothing worthwhile to say.
Use it to point out excellence as well as areas for improvement - the person upon whose code you are commenting will not be the only reader, your other colleagues will be too, and especially good practice should be pointed out to them.