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

[Lars Kurth <lars.kurth@xxxxxxxxxx>]

On 27/11/2019, 19:06, "Stefano Stabellini" <sstabellini@xxxxxxxxxx> wrote:

    On Fri, 27 Sep 2019, 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:
    > 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.
    
    Reading the document, I think Jan has a point that it gives the
    impression that following the suggestions would take significant
    efforts, while actually I don't think Lars meant it that way at all, and
    I don't think it should be the case either.

Yes. Ultimately the effect of a better communication should overall be a 
net-positive in terms of effort. 
    
    Maybe we should highlight and encourage "clarity" instead of "verbosity"
    of the communication, and encourage "expressing appreciation" to
    newcomers, not necessarily to seasoned contributors.

Good idea
    
    The ultimate goal of this document is actually to *reduce* our overall
    efforts by making our communication more efficient, not to increase
    efforts. Maybe it is worth saying this too.
    
It is worth saying this. 

Regards
Lars



_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/mirageos-devel