3 ms·

> Unless there's something extremely interesting, overly cautious phrasing and "positive" comments are mostly noise. Not really. Positive comments help teach e

> Unless there's something extremely interesting, overly cautious phrasing and "positive" comments are mostly noise. Not really. Positive comments help teach engineers which things they've done that conform to local best practices (and why) without them having to meticulously dig those up (assuming they're even documented). A lack of positive comments leaves engineers to learn them only by running afoul of them. Effectively it provides direction only when some threshold of badness is crossed, while leaving positive comments on good code (especially for new team members or junior engineers) provides a beacon pointing away from the badness threshold entirely. Put differently: commenting on good code makes for swifter and less eventful code reviews by steering engineers away from the bad practices that make code challenging to review in the first place. Also, it's just nicer to spend forty hours a week with people who demonstrably appreciate their peers' good work.

mytailorisrich 7y ago
A code review is not the time and place to report positive or convoluted comments. A code review is to find issues and to report them clearly. The only positive thing to report should thus be that all is fine if the reviewer did not find any issues. Now, if you are conducting the review in a meeting then obviously you can make oral comments in passing. If you're using a software tool for reviews then the all the comments should be on point. Nothing prevent you from talking to your coworker afterwards to spread some love if you want to.
- afarrell 7y ago
  > A code review is not the time and place to report positive ... comments. Why not? (Convoluted comments should be made more concise)
  - mytailorisrich 7y ago
    Because a code review is not to pat each other on the back, it is to inspect and report issues. It is already costly enough without going off topic. As said, if you want to praise then you are free to do it offline. This is a professional procedure in a professional setting, not warm words from an encouraging teacher at school... I don't want to have to go through comments that do not add any value to the exercise of finding issues, and I have never seen people leave such comments in 20 years.
    - jsolson 7y ago
      > This is a professional procedure in a professional setting, not warm words from an encouraging teacher at school... Citing a good practice in someone's code as "yes, please do more of this" alongside "don't do this please" is not, in my opinion, fluff. > I don't want to have to go through comments that do not add any value to the exercise of finding issues, and I have never seen people leave such comments in 20 years. So only pay attention to unresolved comments?
      - mytailorisrich 7y ago
        > Citing a good practice in someone's code as "yes, please do more of this" That simply isn't the purpose of a code review. Good practices should be documented externally, so you can check them consistently during code review ;) It's also quite useful to have a checklist when doing a code review.
- jsolson 7y ago
  I disagree, with a caveat. Assuming your review tool supports "Resolved" or "No Action Required" comments (ours does), it's rather easy to distinguish something that is informative from something that is actionable. I am now recalling that some review tools don't distinguish open/unresolved comments from resolved or informative comments, which would make this more of a trade-off than an obvious win.

mytailorisrich 7y ago

A code review is not the time and place to report positive or convoluted comments. A code review is to find issues and to report them clearly. The only positive thing to report should thus be that all is fine if the reviewer did not find any issues. Now, if you are conducting the review in a meeting then obviously you can make oral comments in passing. If you're using a software tool for reviews then the all the comments should be on point. Nothing prevent you from talking to your coworker afterwards to spread some love if you want to.

afarrell 7y ago
> A code review is not the time and place to report positive ... comments. Why not? (Convoluted comments should be made more concise)
- mytailorisrich 7y ago
  Because a code review is not to pat each other on the back, it is to inspect and report issues. It is already costly enough without going off topic. As said, if you want to praise then you are free to do it offline. This is a professional procedure in a professional setting, not warm words from an encouraging teacher at school... I don't want to have to go through comments that do not add any value to the exercise of finding issues, and I have never seen people leave such comments in 20 years.
  - jsolson 7y ago
    > This is a professional procedure in a professional setting, not warm words from an encouraging teacher at school... Citing a good practice in someone's code as "yes, please do more of this" alongside "don't do this please" is not, in my opinion, fluff. > I don't want to have to go through comments that do not add any value to the exercise of finding issues, and I have never seen people leave such comments in 20 years. So only pay attention to unresolved comments?
    - mytailorisrich 7y ago
      > Citing a good practice in someone's code as "yes, please do more of this" That simply isn't the purpose of a code review. Good practices should be documented externally, so you can check them consistently during code review ;) It's also quite useful to have a checklist when doing a code review.
jsolson 7y ago
I disagree, with a caveat. Assuming your review tool supports "Resolved" or "No Action Required" comments (ours does), it's rather easy to distinguish something that is informative from something that is actionable. I am now recalling that some review tools don't distinguish open/unresolved comments from resolved or informative comments, which would make this more of a trade-off than an obvious win.