Home | About | Sematext search-lucene.com search-hadoop.com
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB
 Search Hadoop and all its subprojects:

Switch to Threaded View
Hadoop >> mail # dev >> Requirements for patch review


Copy link to this message
-
Re: Requirements for patch review
I filed HADOOP-8248 with a diff to the bylaws.

Thanks
-Todd

On Wed, Apr 4, 2012 at 3:26 PM, Eli Collins <[EMAIL PROTECTED]> wrote:
> On Wed, Apr 4, 2012 at 2:12 PM, Todd Lipcon <[EMAIL PROTECTED]> 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  and we can
> have quick vote on it here.
>
> Thanks,
> Eli

--
Todd Lipcon
Software Engineer, Cloudera
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB