Re: [PATCH] Documentation/CommunityGuidelines
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 11, 2013, 17:00 UTC
- Message-ID
- <7v38sod1kn.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <51B6AA7F.1060505@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I would prefer a community standards document that looks more like this:
OK.
Show 5 quoted lines
> * 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.
In short, the only acceptable response to review comments are "You are right. Here is a reroll", or "I think your suggestion will miss these cases which I wanted to cover and that is why I did it this way". There may be other small variants of the above two, but I think I can agree with the general principle.
In cases, there are two or more equally valid approaches to solving a problem. I do not think I had to accept (or reject) many "it can be done better in different ways and this perhaps is not the best one" (or "it could be argued that it is correct") borderline topics in the recent past, but I suspect that "a disagreement is healthy" has to be accompanied by how disagreements that do not resolve themselves are resolved (I think I've heard somewhere that some communities break ties in favor of reviewers, for example).
> * 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.
Show 6 quoted lines
> * 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.
Why do you think is it not OK? The "beating" part?