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 ...
For further actions, you may consider blocking this person and/or reporting abuse
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.