[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [PATCH v2 5/6] Add guide on Communication Best Practice
On 27/09/2019, 11:17, "Lars Kurth" <lars.kurth@xxxxxxxxxx> wrote: On 27/09/2019, 10:14, "Jan Beulich" <jbeulich@xxxxxxxx> 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: 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. At the end of the day, none if this is mandatory. The document also has two audiences * Authors which get review feedback : for example by just having this section in there it helps This was meant to read: it helps set expectations and promotes understanding for some of the patterns used I added this section primarily because we do see the occasional very terse review style and even I think sometimes: wow, that comes across as harsh. But I also know, that it isn't intentional and that I have a fairly thick skin. And it is not exclusive to typos and minor issues. Lars _______________________________________________ Minios-devel mailing list Minios-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/minios-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |