4 ms·

Some of my thoughts about code review: See code review as a conversation where both of you can learn something and help each other. Be nice and be kind. Bein

Some of my thoughts about code review: See code review as a conversation where both of you can learn something and help each other. Be nice and be kind. Being nice means asking questions instead of asserting something, taking a neutral stance without needing to resort to overly critical writing. Being kind means caring enough to help the person get it right and learn how and why so they can be better next time. I believe HN can probably come up with some nice guidance for going code reviews.

watwut 3y ago
> See code review as a conversation where both of you can learn something and help each other. I genuinely do not like this. I do not want a conversation here and most of the time I do not want help. I did something, if you have reasonable issue with it, please tell me politely. However, if you want to help or teach me something, talk to me like a normal person. Do not block my PR because you want to teach me something that should be non-blocking.
- tuyiown 3y ago
  > Do not block my PR because you want to teach me something that should be non-blocking. People that do that are not nice nor respectful, I don't think we're in the scope of people wanting to help and learn from each other.
tuyiown 3y ago
Reading your comment, one that entire agree with, I realize that the «review» is pretty loaded, putting the «reviewer» in a place of authority by default. While most of the time, the review occurs when work seemed good enough for the requirements, the effort and cost in time in accord with real world micro economics, meaning that code is submitted by a professional doing the best effort in his judgment, and you better have good reasons for your feedback. In that light, it's pretty clear why things can go sour pretty quickly if a reviewer is not cautious with his feedback, and also why it's hard to make it work if reviews are enforced.
- erikpukinskis 3y ago
  > most of the time, the review occurs when work seemed good enough for the requirements Most of the time, a blocking review should contain information that the coder didn’t have, which changes their own perception about the readiness of the code: a bug they didn’t notice, a pattern in the codebase they didn’t know about, etc. If there is any debate about whether a change is necessary it should probably be a “nit” with an approving review. The point of “request changes” is to flag branches that everyone can agree need changes.
mbork_pl 3y ago
> Be nice and be kind. This reminded me of "Always try to be nice; never fail to be kind." Actually a very good advice (and not all advice from that, well, person is good). I mean: sometimes you can't/shouldn't be nice, but it seems that kindness is universally good (or at least situations when it is not recommended to be kind are less frequent).

watwut 3y ago

> See code review as a conversation where both of you can learn something and help each other. I genuinely do not like this. I do not want a conversation here and most of the time I do not want help. I did something, if you have reasonable issue with it, please tell me politely. However, if you want to help or teach me something, talk to me like a normal person. Do not block my PR because you want to teach me something that should be non-blocking.

tuyiown 3y ago
> Do not block my PR because you want to teach me something that should be non-blocking. People that do that are not nice nor respectful, I don't think we're in the scope of people wanting to help and learn from each other.

tuyiown 3y ago

Reading your comment, one that entire agree with, I realize that the «review» is pretty loaded, putting the «reviewer» in a place of authority by default. While most of the time, the review occurs when work seemed good enough for the requirements, the effort and cost in time in accord with real world micro economics, meaning that code is submitted by a professional doing the best effort in his judgment, and you better have good reasons for your feedback. In that light, it's pretty clear why things can go sour pretty quickly if a reviewer is not cautious with his feedback, and also why it's hard to make it work if reviews are enforced.

erikpukinskis 3y ago
> most of the time, the review occurs when work seemed good enough for the requirements Most of the time, a blocking review should contain information that the coder didn’t have, which changes their own perception about the readiness of the code: a bug they didn’t notice, a pattern in the codebase they didn’t know about, etc. If there is any debate about whether a change is necessary it should probably be a “nit” with an approving review. The point of “request changes” is to flag branches that everyone can agree need changes.

mbork_pl 3y ago

> Be nice and be kind. This reminded me of "Always try to be nice; never fail to be kind." Actually a very good advice (and not all advice from that, well, person is good). I mean: sometimes you can't/shouldn't be nice, but it seems that kindness is universally good (or at least situations when it is not recommended to be kind are less frequent).