4 ms·

> Strive to be intentional, focused, and sparing in our reviews. I get this but I prefer the opposite. By over communicating your preferences, asking question

> Strive to be intentional, focused, and sparing in our reviews. I get this but I prefer the opposite. By over communicating your preferences, asking questions, leaving “if I follow this correctly” comments, and generally not holding back, you create lots of chances for discussion and knowledge sharing. And you’ll catch more bugs. This is especially true when a junior dev is involved.

Conan_Kudo 5y ago
I'm conflicted on this. On one hand, I think it makes sense to over-communicate preferences because building consensus on these things is hard without it. On the other hand, I don't like how it causes everything to drag out... Maybe it's worth trading off on going one way or another? Or maybe there's a happy medium somewhere...
- cube2222 5y ago
  A nice practice is to finish all such nitpicky comments with the strength of your opinion / how much you care. This way, if most of those are "very little" then it's up to the author to decide whether to do it or not, and they don't have to wait for you to circle back about it.
- LukeShu 5y ago
  The happy medium I've found is to clarify my level of preference; whether it's a "nitpick" or an actual "change request". I communicate that you don't have to update the PR just for nitpicks, but that if you're updating it anyway that they might be good things to also change.
matt7340 5y ago
I agree with the spirit of this, but the async nature of code review seems to prevent it. Social and power dynamics in async code review can make things very difficult.
- sesuximo 5y ago
  This is a big factor. I’ve experimented with in person code reviews but it’s kind of annoying to organize. Definitely open to ideas on this front.
watwut 5y ago
Imo, the code review is not the place to over communicate your preferences. Do it on developers meeting. Do it in a chat. Do it on standup. Don't do it in random code reviews where people unlucky enough to get you as reviewer have to deal with different expectations then anybody else and spend hours discussing your personal preferences - without any real impact on general codebase because other reviewers and coders have different preferences.
- sesuximo 5y ago
  Maybe preference was the wrong word for me to use. “Reasoning” might make more sense? If comments don’t impact the code base or the people involved, then yeah they’re not useful.

Conan_Kudo 5y ago

I'm conflicted on this. On one hand, I think it makes sense to over-communicate preferences because building consensus on these things is hard without it. On the other hand, I don't like how it causes everything to drag out... Maybe it's worth trading off on going one way or another? Or maybe there's a happy medium somewhere...

cube2222 5y ago
A nice practice is to finish all such nitpicky comments with the strength of your opinion / how much you care. This way, if most of those are "very little" then it's up to the author to decide whether to do it or not, and they don't have to wait for you to circle back about it.
LukeShu 5y ago
The happy medium I've found is to clarify my level of preference; whether it's a "nitpick" or an actual "change request". I communicate that you don't have to update the PR just for nitpicks, but that if you're updating it anyway that they might be good things to also change.

matt7340 5y ago

I agree with the spirit of this, but the async nature of code review seems to prevent it. Social and power dynamics in async code review can make things very difficult.

sesuximo 5y ago
This is a big factor. I’ve experimented with in person code reviews but it’s kind of annoying to organize. Definitely open to ideas on this front.

watwut 5y ago

Imo, the code review is not the place to over communicate your preferences. Do it on developers meeting. Do it in a chat. Do it on standup. Don't do it in random code reviews where people unlucky enough to get you as reviewer have to deal with different expectations then anybody else and spend hours discussing your personal preferences - without any real impact on general codebase because other reviewers and coders have different preferences.

sesuximo 5y ago
Maybe preference was the wrong word for me to use. “Reasoning” might make more sense? If comments don’t impact the code base or the people involved, then yeah they’re not useful.