incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Todd Lipcon <>
Subject Re: RTC vs CTR (was: Concerning Sentry...)
Date Fri, 20 Nov 2015 18:50:14 GMT
On Thu, Nov 19, 2015 at 6:49 PM, Greg Stein <> 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.


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