Mailing-List: contact common-dev-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Received-SPF: pass (nike.apache.org: domain of todd@cloudera.com designates
 209.85.214.176 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAPn_vTsroydZE5M83kFR4Xkj3Om2gQYSEmLOKuc81+KFQsumXg@mail.gmail.com>
References: 
 <CADY20s6gh7qJBDbVfXDGrJGs4Qst5iaSN9vnipmefTMrQ91NEA@mail.gmail.com>
 <CAPn_vTsroydZE5M83kFR4Xkj3Om2gQYSEmLOKuc81+KFQsumXg@mail.gmail.com>
From: Todd Lipcon <todd@cloudera.com>
Date: Wed, 4 Apr 2012 18:13:37 -0700
Message-ID: 
 <CADY20s5B=uz_qVtkcgjd4q2DU7wsEgQXq7=5E0YHwMo9FreYSA@mail.gmail.com>
Subject: Re: Requirements for patch review
To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

I filed HADOOP-8248 with a diff to the bylaws.

Thanks
-Todd

On Wed, Apr 4, 2012 at 3:26 PM, Eli Collins <eli@cloudera.com> wrote:
> On Wed, Apr 4, 2012 at 2:12 PM, Todd Lipcon <todd@cloudera.com> wrote:
>> Hi folks,
>>
>> Some discussion between Nicholas, Aaron, and me started in the
>> comments of HDFS-3168 which I think is better exposed on the mailing
>> list instead of trailing an already-committed JIRA.
>>
>> The question at hand is what the policy is with regarding our
>> review-then-commit policies. The bylaws state:
>>
>>>>>
>> *Code Change*
>> A change made to a codebase of the project and committed by a
>> committer. This includes source code, documentation, website content,
>> etc. Lazy consensus of active committers, but with a minimum of one
>> +1. The code can be committed after the first +1, unless the code
>> change represents a merge from a branch, in which case three +1s are
>> required.
>> <<<
>>
>> The wording here is ambiguous, though, whether the committer who
>> provides the minimum one +1 may also be the author of the code change.
>> If so, that would seem to imply that committers may always make code
>> changes by merely +1ing their own patches, which seems counter to the
>> whole point of "review-then-commit". So, I'm pretty sure that's not
>> what it means.
>>
>> The question that came up, however, was whether a non-committer
>> contributor may provide a binding +1 for a patch written by a
>> committer. So, if I write a patch as a committer, and then a community
>> member reviews it, am I free to commit it without another committer
>> looking at it? My understanding has always been that this is not the
>> case, but we should clarify the by-laws if there is some ambiguity.
>>
>> I would propose the following amendments:
>> A committer may not provide a binding +1 for his or her own patch.
>> However, in the case of trivial patches only, a committer may use a +1
>> from the problem reporter or other contributor in lieu of another
>> committer's +1. The definition of a trivial patch is subject to the
>> committer's best judgment, but in general should consist of things
>> such as: documentation fixes, spelling mistakes, log message changes,
>> or additional test cases.
>>
>> I think the above strikes a reasonable balance between pragmatism for
>> quick changes, and keeping a rigorous review process for patches that
>> should have multiple experienced folks look over.
>>
>> Thoughts?
>>
>
> Sounds reasonable to me.
>
> Maybe file a jira with the proposed diff to the bylaws xml =A0and we can
> have quick vote on it here.
>
> Thanks,
> Eli


--=20
Todd Lipcon
Software Engineer, Cloudera