On 9/27/19 10:14 AM, Jan Beulich wrote: > On 26.09.2019 21:39, Lars Kurth wrote: >> +### Verbose vs. terse >> +Due to the time it takes to review and compose code reviewer, reviewers >> often adopt a >> +terse style. It is not unusual to see review comments such as >> +> typo >> +> s/resions/regions/ >> +> coding style >> +> coding style: brackets not needed >> +etc. >> + >> +Terse code review style has its place and can be productive for both the >> reviewer and >> +the author. However, overuse can come across as unfriendly, lacking empathy >> and >> +can thus create a negative impression with the author of a patch. This is >> in particular >> +true, when you do not know the author or the author is a newcomer. Terse >> +communication styles can also be perceived as rude in some cultures. > > And another remark here: Not being terse in situations like the ones > enumerated as examples above is a double waste of the reviewer's time:
FWIW I don't think this document prohibits terse replies. It points out that they can come across as unfriendly, and they can be perceived as rude in some cultures; both of which are true. It then *recommends* that reviewers compensate for it in a review opening (i.e., once per patch / series) which expresses appreciation; which is both helpful and relatively low cost. The point of the opening is to set the tone. If you start out with something positive, and ends with "thanks", then a long series of terse comments is more likely to be read as simply being efficient. If the entire review consists of nothing but criticism or terse comments, it's more likely to be read as annoyance on the part of the reviewer. > They shouldn't even need to make such comments, especially not many > times for a single patch (see your mention of "overuse"). I realize > we still have no automated mechanism to check style aspects, but > anybody can easily look over their patches before submitting them. > And for an occasional issue I think a terse reply is quite reasonable > to have. This sort of sounds like you are *intending* to express annoyance? If so, that's a slightly different question than what this section is addressing. :-) -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
