hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Evans <ev...@yahoo-inc.com>
Subject Re: Requirements for patch review
Date Wed, 04 Apr 2012 22:09:52 GMT
I personally like the clarification and it is in line with how I understood the original bylaw
when I read it.  I don't really want this to turn into a legal document but as this is getting
more explicit with clarification it would be nice to put in a small exception for release
managers when they are changing versions and setting up a new release branch.

--Bobby Evans

On 4/4/12 4: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?

Todd
--
Todd Lipcon
Software Engineer, Cloudera


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message