I have now, days later, Reviewed this Thread and Commit to a veto of the whole debate, Can't agree That it is Rewarding for anyone... ;-)
On Sat, Nov 21, 2015 at 2:50 AM, Todd Lipcon <t...@apache.org> wrote: > On Thu, Nov 19, 2015 at 6:49 PM, Greg Stein <gst...@gmail.com> wrote: > > > Todd: as Ross notes, your three points about code reviews in a CTR > project > > are quite valid, and match accepted engineering practices. > > > > What I haven't seen is an explanation why a committer must be treated the > > same as a drive-by. Both are subject to requiring "permission"[1] to make > > even the simplest of changes under RTC. Even worse, from else-thread, it > > sounds like under your control scheme, you don't even allow the committer > > to apply their own change [after review]. > > > They can apply their own change once someone else has +1ed it. On Hadoop, > for example, the usual workflow when I review another committer's patch is > that I give them a +1 and they commit it themselves. On gerrit or github, > because the actual "commit" process is just clicking a button on a web UI, > it's more normal for the reviewer to be the one to commit it after giving > the +1 review, but both happen and either one's fine. > > > > A committer can give a binding +1 > > to somebody else's work. But they aren't trusted to give that to their > own > > work. Just like a drive-by contributor can't be trusted. > > > > Right, they can't give it to their own work because it defeats the purpose > of the code review (discussed earlier). > > Of course it's not hard and fast -- eg fixing a broken build due to a > missing 'import' statement or something would be totally fine to commit > without review, or fixing a grammar mistake in a comment, or anything else > that's obviously trivial. But once actual code is changing, it's expected > to get two pairs of eyes. > > -Todd > -- Niclas Hedhman, Software Developer http://zest.apache.org - New Energy for Java
