incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Todd Lipcon <t...@cloudera.com>
Subject Re: RTC vs CTR (was: Concerning Sentry...)
Date Thu, 19 Nov 2015 17:10:52 GMT
On Thu, Nov 19, 2015 at 8:16 AM, Ralph Goers <ralph.goers@dslextreme.com>
wrote:

> None of your statements below are any different between RTC or CTR. The
> only time it makes aa difference is if no one does reviews.  My feeling is
> that a community that insists on RTC believes that code will not be
> reviewed unless committers are forced to do it.
>

Yep, that's my assumption. It's much more fun to write code than review it,
so "forcing" people to do it is a good idea. The other worry is that, in a
large community of developers, an implicit "people probably read the
commits as they come in" doesn't scale. Should every person read every
commit? Probably not. How do you know if someone else has already read the
commit, or signed up to do so? What if it takes you a few days to get
around to reviewing something, and by that point there are already a bunch
more patches stacked up on top making it impossible to revert or difficult
to modify? Who's in charge of fixing the bugs or issues in a post-commit
review?

I'm sure it works fine for many communities (I use CTR on some internal
infrastructure within small teams where bugs are less costly and the rate
of development is slow). But it doesn't work for all.


>
> All I can say, is that for me personally I have found the process of
> having to create a patch, submit a code review, wait for the review and
> participate in it, then wait for the commit to be onerous enough that I
> just don’t bother.


Sure, that's a big problem with some RTC workflows. Using gerrit or github
PRs makes the flow much easier -- for a trivial or small patch, the sort
that a "drive-by" contributor typically contributes, there probably won't
be any review comments. So, they just push the patch for review, and they
can be out of the loop for the rest of it. If the patch requires small
revisions (eg addressing a typo or something) I think it's fine for the
reviewer to just make the change themselves and commit on behalf of the
original author to avoid the issue you've raised. Most RTC workflows permit
this kind of thing in my experience.


> As I said, in a CTR community there are many times where branches are
> created and the code is reviewed there before being merged because the
> authors believe the code is significant enough to require it.


Amusingly enough, the RTC communities I'm a part of do the opposite: you
can make a branch which operates under CTR, so long as it's reviewed
sufficiently prior to its merge into trunk. This is great for rapid
development and prototyping when a small number of contributors are working
together on a new project.

-Todd
-- 
Todd Lipcon
Software Engineer, Cloudera

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