Mailing-List: contact common-dev-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Received-SPF: pass (athena.apache.org: local policy)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
  s=s1024; d=yahoo.com;
  h=X-YMail-OSG:Received:X-Mailer:References:Message-ID:Date:From:Reply-To:Subject:To:In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding;
  b=nkcK1HvkanHEtewnyQErV5unN12rrXFeA6opcrihRrRtjGmb5Xt5Hzy+/WSdL2jAgPH2ZfNxUwHiwmsnPjYJ+g3cHgswYp92NMv0/dCCFhpaqjoK6vHAb60Cbt7ygw/NLvLflIZMKvY6HITh9bs9wvCT0R0r+vO+5ir4+5sjPGA=;
References: 
 <CADY20s6gh7qJBDbVfXDGrJGs4Qst5iaSN9vnipmefTMrQ91NEA@mail.gmail.com>
 <CAPn_vTsroydZE5M83kFR4Xkj3Om2gQYSEmLOKuc81+KFQsumXg@mail.gmail.com>
 <CADY20s5B=uz_qVtkcgjd4q2DU7wsEgQXq7=5E0YHwMo9FreYSA@mail.gmail.com>
 <1333596274.24445.YahooMailNeo@web65902.mail.ac4.yahoo.com>
Message-ID: <1333642447.59884.YahooMailNeo@web65901.mail.ac4.yahoo.com>
Date: Thu, 5 Apr 2012 09:14:07 -0700 (PDT)
From: "Tsz Wo \(Nicholas\), Sze" <s29752-hadoopdev@yahoo.com>
Reply-To: "Tsz Wo \(Nicholas\), Sze" <s29752-hadoopdev@yahoo.com>
Subject: Fw: Requirements for patch review
To: common dev <common-dev@hadoop.apache.org>
In-Reply-To: <1333596274.24445.YahooMailNeo@web65902.mail.ac4.yahoo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable

Resent.=0A=0A=0A=0A----- Forwarded Message -----=0AFrom: Tsz Wo Sze <szetsz=
wo@yahoo.com>=0ATo: "common-dev@hadoop.apache.org" <common-dev@hadoop.apach=
e.org>=0ACc: =0ASent: Wednesday, April 4, 2012 8:24 PM=0ASubject: Re: Requi=
rements for patch review=0A=0A>> The wording here is ambiguous, though, whe=
ther the committer who=0A>> provides the minimum one +1 may also be the aut=
hor of the code change.=0A>> If so, that would seem to imply that committer=
s may always make code=0A>> changes by merely +1ing their own patches, whic=
h seems counter to the=0A>> whole point of "review-then-commit". So, I'm pr=
etty sure that's not=0A>> what it means.=0A>>=0A>> The question that came u=
p, however, was whether a non-committer=0A>> contributor may provide a bind=
ing +1 for a patch written by a=0A>> committer. So, if I write a patch as a=
 committer, and then a community=0A>> member reviews it, am I free to commi=
t it without another committer=0A>> looking at it? My understanding has alw=
ays been that this is not the=0A>> case, but we should clarify the by-laws =
if there is some ambiguity.=0A>>=0A>> I would propose the following amendme=
nts:=0A>> A committer may not provide a binding +1 for his or her own patch=
.=0A>> However, in the case of trivial patches only, a committer may use a =
+1=0A>> from the problem reporter or other contributor in lieu of another=
=0A>> committer's +1. The definition of a trivial patch is subject to the=
=0A>> committer's best judgment, but in general should consist of things=0A=
>> such as: documentation fixes, spelling mistakes, log message changes,=0A=
>> or additional test cases.=0A=0AI agree that the bylaws is not clear abou=
t this.=A0 For reviewing patches, my understanding is that any contributor,=
 a committer or not, could review patches and the +1 counts.=A0 I have work=
ed on Hadoop almost five years.=A0 This is what we are doing for a long tim=
e (if it is not from the beginning of the Hadoop project.)=A0 Could other p=
eople confirm this?=0A=0AFrom the HowToContribute wiki, it does advise comm=
itters to find another committer to review difficult patches: "Committers: =
for non-trivial changes, it is best to get another committer to review your=
 patches before commit. ..."=A0 It seems saying that it is okay for non-com=
mitters reviewing simple and medium patches.=A0 Todd's amendments use diffe=
rent wording which seems implying a different requirement: the +1's from no=
n-committers could be counted only for simple patches but not medium and di=
fficult patches.=0A=0AI think we should keep allowing everyone to review pa=
tches.=A0 It slows down the development and is discouraging if non-committe=
r's +1 does not count.=A0 I believe the judgement of the committer who comm=
its the patch won't commit bad code.=A0 We have svn and we could revert pat=
ches if necessary.=A0 Lastly, if a committer keeps committing bad code, we =
could exercise "Committer Removal".=0A=0ABTW, does anyone know what other A=
pache projects do?=0A=0APS: since this is a bylaws change discussion, shoul=
d we discuss it in general@?=0A=0ARegards,=0ATsz-Wo=0A