Resent.
----- Forwarded Message ----- From: Tsz Wo Sze <szets...@yahoo.com> To: "common-dev@hadoop.apache.org" <common-dev@hadoop.apache.org> Cc: Sent: Wednesday, April 4, 2012 8:24 PM Subject: Re: Requirements for patch review >> 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 agree that the bylaws is not clear about this. For reviewing patches, my understanding is that any contributor, a committer or not, could review patches and the +1 counts. I have worked on Hadoop almost five years. This is what we are doing for a long time (if it is not from the beginning of the Hadoop project.) Could other people confirm this? From the HowToContribute wiki, it does advise committers 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. ..." It seems saying that it is okay for non-committers reviewing simple and medium patches. Todd's amendments use different wording which seems implying a different requirement: the +1's from non-committers could be counted only for simple patches but not medium and difficult patches. I think we should keep allowing everyone to review patches. It slows down the development and is discouraging if non-committer's +1 does not count. I believe the judgement of the committer who commits the patch won't commit bad code. We have svn and we could revert patches if necessary. Lastly, if a committer keeps committing bad code, we could exercise "Committer Removal". BTW, does anyone know what other Apache projects do? PS: since this is a bylaws change discussion, should we discuss it in general@? Regards, Tsz-Wo
