Re: [PATCH] Documentation/CommunityGuidelines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/11/2013 07:00 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
> [...]
>> * When reviewing other peoples' code, be tactful and constructive.  Set
>> high expectations, but do what you can to help the submitter achieve
>> them.  Don't demand changes based only on your personal preferences.
>> Don't let the perfect be the enemy of the good.
> 
> I think this is 30% aimed at me (as I think I do about that much of
> the reviews around here).  I fully agree with most of them, but the
> last sentence is a bit too fuzzy to be a practically useful
> guideline.  Somebody's bare minimum is somebody else's perfection.
> An unqualified "perfect is the enemy of good" is often incorrectly
> used to justify "It works for me." and "There already are other
> codepaths that do it in the same wrong way.", both of which make
> things _worse_ for the long term project health.

I agree that the last line is fuzzy.  And I don't think that I've
observed any cases where I thought that reviewers were being too strict,
so in a way it's just trying to head off hypothetical future problems
and to make sure that the balance between submitter and reviewer is not
*entirely* one-sided.  Given our (proper, I think) strong deference to
reviewers, one could imagine a reviewer abusing his/her authority to
obstruct reasonable changes by (for example) making demands that the
submitter also fix tangentially-related things that are beyond the scope
of the patch.

In my own projects I have a rough policy of "not worse than before",
meaning that as long as a patch makes progress in at least one
dimension, and doesn't make things worse in any other dimension, then it
is acceptable.  (Of course "worse" can include internal quality issues
like copy-pasting code or even an increase in the amount of code
disproportionate to its benefit.)  A failure to make improvements in one
area should not be a reason to block an improvement in another area, as
long as nothing is made worse.

But I can't right now think of a succinct way to express what I have in
mind.

>> * It is not OK to use these guidelines as a stick with which to beat
>> supposed violators.  However, if you genuinely feel that another
>> community member is routinely behaving in ways that are detrimental to
>> the community, it might help to calmly express your concerns to that
>> person, preferably in a private email, and naming concrete and specific
>> incidents rather than broad generalizations.
> 
> I would think it is perfectly OK to say "The way you are refusing to
> listen to constructive comments is not how things work around here"
> by pointing at a set of guidelines.

I agree.

> Why do you think is it not OK?  The "beating" part?

I think it would be counterproductive for people to start saying things
like "that is a violation of rule 3, section 2" *in everyday
discussions*.  This shouldn't be taken as a list of black-and-white
laws, with allegations of small "infractions" used to shut down
discussions.  And on the other hand, if somebody shows a long history of
acting contrary to the guidelines, and persists despite repeated
requests to stop, I don't want the discussion to turn into a lawyerly
analysis of the guidelines with point-by-point rebuttals and
counter-rebuttals of whether this or that guideline was violated.

The guidelines should just describe the expected tone of the community
in a way that the vast majority of participants can agree on, and any
kind of actions to enforce the guidelines should only be taken when an
overwhelming majority of the community

I think the CommunityGuidelines should have three main uses:

1. An artifact documenting the community consensus about what kinds of
behaviors are encouraged and what kinds are considered unacceptable.  It
should only be accepted, and it only has value, if there is a strong
consensus in favor of it.

2. A resource to help new community members get up to speed on our
practices and expectations.

3. As a point of reference in the direst meltdowns, such as IMO we are
having right now.

Michael

-- 
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]