incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Todd Lipcon <t...@apache.org>
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 <gstein@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

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