So. After this long debate, it seems like no one is on either extreme of the CTR/RTC spectrum. The entire debate seems to be exactly where within the spectrum one likes to be.
It seems reasonable that the exact position varies from project to project or even within different pieces within a project… If there is a disagreement, it seems to be part semantics, part version control technologies (i.e. SVN optimized workflow vs Git optimized workflow) and part an actual difference in how to handle certain situations. It seems to me that the actual disagreement is pretty small. ;-) Harbs On Nov 20, 2015, at 8:50 PM, 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 --------------------------------------------------------------------- To unsubscribe, e-mail: general-unsubscr...@incubator.apache.org For additional commands, e-mail: general-h...@incubator.apache.org
