Re: [PATCH] Documentation/CommunityGuidelines

[Thomas Rast <trast@inf.ethz.ch>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> * Accept reviewers' comments gratefully and take them very seriously.
> Show that you appreciate the help by giving the reviewer the benefit of
> the doubt.  If, after careful consideration, you find that you cannot
> agree with a reviewer's suggestion, explain your reasoning carefully
> without taking or giving offense, and seek compromise.

I'm not sure yet how to phrase it, but I have come to the conclusion
that the dual nature of reviews is part of the problem.

On the one hand, reviews are code criticism: they are extra work spent
by the reviewer to try and help you improve your work.

On the other hand, they are also quality checks.  Reviews serve to spot
bugs, misdesigns, noncompliance with project standards, etc. before they
ever go into the code base.

The problems start when these start having a contradictory impact on the
correct course of action in a discussion, or in the longer term in
dealing with a person.  For example, I have attempted to deal with
Felipe at one point by refusing to review, i.e., ignore the email.

However, this must be weighed against the requirements of the second
kind of review.  So while it is exceedingly easy for everyone to say
"just ignore the flamebait", the flamewars keep recurring because this
"gatekeeper" type of review continues to be necessary, and continues to
elicit nonconstructive responses.

The "easy" solution is to simply stop taking patches from Felipe, but
opens pandora's box w.r.t. the just application of such a measure, as
Ram has noted repeatedly.

Yet that is the only measure that I currently know that both keeps up
code review standards and has any hope of improving the current climate.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch