3 ms·
Actually reaching a level of team culture where you can be as brief, blunt and to the point as possible is a wonderful thing. There is no offence to be taken,
by mjfisher 4y ago
Actually reaching a level of team culture where you can be as brief, blunt and to the point as possible is a wonderful thing.
There is no offence to be taken, because the deeply held respect for each other as team mates is already there and taken as a given. You trust each other to know that any comments are addressed directly to the code, rather than to the person behind them. All input is valued because everyone knows the goal is just to make the code better and help each other out, rather than criticise an individual. Discussion can be frank, open, unbiased and non-confrontational.
However, because that takes a long time to develop and is somewhat of a rarity, I've had quite good luck with the phrasing:
"Have you considered X here?"
It has a number of advantages:
* It doesn't imply that an author should have done it a different way
* It doesn't imply that an author has missed something you think they should have covered
* It's a good starter for a conversation, rather than a conflict
* If the answer is "no", it provides a good prompt for an author to come up with a suggestion first
* If the answer is "yes", they can explain why they ended up with the solution they did, which is visible to everyone who wants to look at the PR. That can also be a good prompt to add a comment
- omegalulw 4y agoAnother method: prepend "nit: " to comments that improve the code but are not calling for changing the main logic. As a reviewer, you are simply acknowledging that this is a nitpick :)