incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ross Gardler <Ross.Gard...@microsoft.com>
Subject RE: RTC vs CTR (was: Concerning Sentry...)
Date Thu, 19 Nov 2015 17:46:55 GMT
Todd asks "How do you know if someone else has already read the commit" - I don't care. Just
as people writing code can make mistakes, so can people who review code. For this reason if
*I* care about *that* commit I will review it in detail - regardless of RTC or CTR. 

The more people who follow that rule the more eyes there are on the commit. I trust my fellow
community members to do the right thing.

RTC provides no added value for me personally since the amount of work for *me* is the same
- I need to review every commit I care about.

Ross

-----Original Message-----
From: Todd Lipcon [mailto:todd@cloudera.com] 
Sent: Thursday, November 19, 2015 9:11 AM
To: general@incubator.apache.org
Subject: Re: RTC vs CTR (was: Concerning Sentry...)

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
View raw message