Code reviews do not only improve the quality of code but are a great tool to learn from others and share experience. Doing and having code reviews is not an easy task. Pointing to or criticising your colleagues and peers is not something someone would like to do daily. Therefore, doing it in a constructive and positive manner would be beneficial to both the reviewer and reviewee.
Prepare:
1) Adhere to coding guideline or make one with your team members.
Use the right words:
2) Avoid using negative words and comments like "Remove", "Change", "Not needed", "No", "Bad", "I don't like this" etc...
3) Don't use sarcasm.
4) Avoid referring to code as "mine", "yours", or "theirs".
5) Ask questions instead of giving orders. Use questions like "Do you think this would be better?", "What do you think about...?"
6) Be positive. Don't say words like "I didn't do this", "It wasn't me" etc...
Clarify:
7) Explain the reason behind your comment and how it will improve the code.
8) Express yourself clearly and provide arguments. If needed, link to other code snippets or PRs.
9) Ask for clarification if the code is unclear. If there are too many questions it would be better asking them in person.
10) Be clear in the feedback. If a lot of discussion is needed it can be done in a different session.
Accept:
11) Learn from the comments and how others tackle the same problem.
12) Don't take it personal, code reviews are about the code and not you as a person.
13) Accept all feedback. Whether you are a Junior or Senior take feedback positively.
14) Be open to other ways of writing code or methods.
15) Don't turn the feedback into an ongoing argument. You can always talk to the commenter in person.
Support:
16) Be supportive. Help by pair-programming or showing an example.
17) Be responsive. Provide more information or clarification in the same day or after one day.
18) Be quick. Don't leave too many Pull Requests open. The codebase will grow, and the PR will stay behind.
19) Give positive feedback when the PR is accepted, like using the Like emoji or saying words like "Great, well done".
20) If a feedback helps, inform the reviewer how you find that helpful. Use words like "Good catch, thank you" etc...
Having a constructive and positive code review will foster a healthy work culture and allow team members to express themselves freely without the worry of shame and negative feedback. This will create an environment where team members feel respected, valued and productive. At the end it will allow an agile environment, knowledge sharing and growth.
Top comments (8)
PR reviews are the best indicator of the health status of a dev team. When there is trust and collegiality, feedback flows naturally and the amount of B.S is low. When the components of the team don't work well together, there's people thinking for 10 minutes how to sugar-coat a comment in a PR or maybe whether they should just ignore it and press that big APPROVE button.
At the same time PRs do break relationships, and this is true because a decent PR often has many hours if not days (if not weeks) of hard work behind them so it is natural for a dev to get attached to their work. When someone contradicts your work, you won't like it.
But what definitely gets people offended (and this is something this article should cover) is having comments about their coding style, rather than the functionality or maybe some edge case / error handling. Unless your project has a clear coding guideline which outlines what is the preferred case, be flexible: your opinion should remain your opinion.
If you want to know how I review my PRs, I divide my comments in 3 categories:
Personal preference: "This is just a personal preference but ..." (e.g variable name, alternative method...)
Minor: "Minor: this method could be broken into 2 submethods to make it more readable. Please consider refactoring it"
Major: "Major: null safety - this could cause an exception if the input is null, please verify the input"
Language is everything. For Personal preference comments I don't ask for anything, for Minor comments I politely ask them to consider it but it's up to them, and for Major comments I am no longer asking: it needs to be resolved prior to approval.
The main idea here is to still provide feedback while letting people deliver and be at home in time for playing with their cats (or whatever it is that they do) :).
I absolutely agree. Breaking up the comments into different categories certainly helps in handling PR reviews. In all cases, attitude is key. If the feedback is positively conveyed, even minor or "personal preference" feedback will be constructive and taken delightedly into account.
I recently watched a very interesting presentation by Erin Meyer:
And one of key differences between cultures was taking negative feedback, which I think is a large part of code reviews. Some take it indirectly (3 good things, one bad), some take it directly (literally telling what's bad).
As an example, I take negative feedback directly and expect someone to tell how I can improve without too many hassling. Heck, I could even miss what reviewer wanted to say to me because I was too excited about things I did well totally ignoring my mistakes.
But if I'd critique someone, who expects to hear indirect negative feedback, directly, he might feel crushed and it'd seem that everything's bad and life's over.
This isn't obvious at first, but the more you work with people from different countries, the more you have to adapt and be aware or make them aware of your intentions.
This video is interesting! I guess I have to read her book now, thanks for sharing :)
Definitely culture is a key difference in code reviews and feedback; I also had experienced this personally. It takes time to understand and become aware of, especially when working remotely or when not having met the team in person.
Yep! Reviewer could have best intentions to help you out but sound like an asshole without even realizing that.
I think empathy is an important factor when doing any code review, because it is not necessary that the person before you thinks in the same way as you do while solving a problem.
Another factor is experience, as your experience increases, so does the way how you write the code, so that factor should also be considered.
My code review once was done by a principal architect, and although all his points were valid, he completely did in a negative way cross questioning and showing everything which I did was wrong, this impacts a developer's confidence in a big way.
True. In some cases, people forget that they are dealing with another human, and they comment as if they are thinking out loud. Sometimes this happens involuntarily, like in times of a deadline day, which is Okay I guess. But having this attitude consistent on a regular bases is an indicator of poor dev team culture.
In my opinion, I practice constructive, non offensive and pretty clear why I'm questioning the choices made by the developer in the code reviews. That is why I think it is very impeach on how the company or people working on the project see code review.
Some developers out there, even when you make your good intention clear and hope for the best of the project, cannot help to proove the fact that they do not want code review, they just want the necessary approval from the team.
A process like this must also be presented from the start as a good and useful thing, and something that everyone should be in both sides of the code review as often that they can.