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: 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. Overall I'm seeing the good intentions of this document, yet I'd still vote at least -1 on it if it came to a vote. Following even just a fair part of it is a considerable extra amount of time to invest in reviews, when we already have a severe reviewing bottleneck. If I have to judge between doing a bad (stylistically according to this doc, not technically) review or none at all (because of time constraints), I'd favor the former. Unless of course I'm asked to stop doing so, in which case I'd expect whoever asks to arrange for the reviews to be done by someone else in due course. I'm sorry for (likely) sounding destructive here. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel