Re: [Xen-devel] [PATCH v2 5/6] Add guide on Communication Best Practice

[Lars Kurth <lars.kurth@citrix.com>]

On 27/09/2019, 11:17, "Lars Kurth" <lars.kurth@citrix.com> wrote:

    
    
    On 27/09/2019, 10:14, "Jan Beulich" <jbeulich@suse.com> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel