Re: [LSF/MM TOPIC] Patch Submission process and Handling Internal Conflict

Goldwyn Rodrigues
2018-01-26T20:42:01Z

On 01/24/2018 01:05 PM, James Bottomley wrote:
> I've got two community style topics, which should probably be discussed
> in the plenary
>
> 1. Patch Submission Process
>
> Today we don't have a uniform patch submission process across Storage,
> Filesystems and MM. A The question is should we (or at least should we
> adhere to some minimal standards). A The standard we've been trying to
> hold to in SCSI is one review per accepted non-trivial patch. A For us,
> it's useful because it encourages driver writers to review each other's
> patches rather than just posting and then complaining their patch
> hasn't gone in. A I can certainly think of a couple of bugs I've had to
> chase in mm where the underlying patches would have benefited from
> review, so I'd like to discuss making the one review per non-trival
> patch our base minimum standard across the whole of LSF/MM; it would
> certainly serve to improve our Reviewed-by statistics.
>
> 2. Handling Internal Conflict
>
> My observation here is that actually most conflict is generated by the
> review process (I know, if we increase reviews as I propose in 1. we'll
> increase conflict on the lists on the basis of this observation), so
> I've been thinking about ways to de-escalate it. A The principle issue
> is that a review which doesn't just say the patch is fine (or fine
> except for nitpicks) can be taken as criticism and criticism is often
> processed personally. A The way you phrase criticism can have a great
> bearing on the amount of personal insult taken by the other party.
> A Corny as it sounds, the 0day bot response "Hi Z,A I love your patch!
> Perhaps something to improve:" is specifically targetted at this
> problem and seems actually to work. A I think we could all benefit from
> discussing how to give and receive criticism in the form of patch
> reviews responsibly, especially as not everyone's native language in
> English and certain common linguistic phrasings in other languages can
> come off as rude when directly translated to English (Russian springs
> immediately to mind for some reason here). A Also Note, I think fixing
> the review problem would solve most of the issues, so I'm not proposing
> anything more formal like the code of conflict stuff in the main
> kernel.

The problem I have faced is that reviewers usually (not generalizing)
review by saying "This is bad", "This is not acceptable" or "This will
not work" and leaving it there. Instead a reviewer should be focused on
providing the alternates and/or reasons like "This is bad because ..."
or "This will not work. Instead you should be doing ..." In short
towards constructive criticism, providing what reviewers think how it
should be done or resolved to move forward. While providing too much
information may be called spoon-feeding, there is always a balance..

>
> We could lump both of these under a single "Community Discussion" topic
> if the organizers prefer ... especially if anyone has any other
> community type issues they'd like to bring up.
>
> James
>
>

--
Goldwyn