hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Purtell <andrew.purt...@gmail.com>
Subject Re: [DISCUSSION] Switching from RTC to CTR
Date Wed, 29 Jul 2015 21:09:50 GMT
@dev is now CCed

I didn't want to over structure the discussion with too much detail up front. I do think CTR
without supporting process or boundaries can be more problematic than helpful. That could
take the form of customarily waiting for reviews before commit even under a CTR regime. I
think this discussion has been great so far. I would also add that CTR moves 'R' from a gating
requirement per commit (which can be hard to overcome for niche areas or when volunteer resources
are less available) more to RMs. will be back later with more. 

> On Jul 29, 2015, at 1:36 PM, Sean Busbey <sean.busbey@gmail.com> wrote:
> I'd also favor having this discussion on dev@.
>> On Wed, Jul 29, 2015 at 2:29 PM, Gary Helmling <ghelmling@gmail.com> wrote:
>> This is already a really interesting and meaningful discussion, and is
>> obviously important to the community.
>> Is there any reason not to move this straight to the dev@ list?
>>> On Wed, Jul 29, 2015 at 11:40 AM Todd Lipcon <todd@cloudera.com> wrote:
>>> I'm not very active in HBase these days, so I won't cast a non-zero vote,
>>> but I'm -0 on this idea, for basically two reasons:
>>> 1) In my experience at a past job which used CTR, the reality was that it
>>> was more like "Commit and maybe review" rather than "Commit then review".
>>> It's always more fun (and often easier) to write new code than to review
>>> other people's code, so if there isn't a requirement that all code gets
>>> reviewed before commit, it's easy for the ratio of code written to code
>>> reviewed to tend towards values significantly greater than 1:1 over time.
>>> At my past job, this meant that a lot of code made it into production (it
>>> was a website) that hadn't ever been reviewed, and in many cases we found
>>> bugs (or other issues) that would have definitely been caught by a good
>>> code reviewer.
>>> 2) CTR has both the advantage and disadvantage of allowing areas of code
>> to
>>> be evolved quickly by a single person. That could be seen as a plus, in
>>> that there are some areas which tend to get ignored because we don't
>> have a
>>> critical mass of people reviewing patches in the area -- patches languish
>>> in these areas currently. However, that could also be seen as a good
>> thing
>>> from a "community over code" perspective -- it's better to not have any
>>> areas of code with bus-factor 1. Feeling the pain of not getting reviews
>> in
>>> these areas with only a single active committer encourages us to find
>>> solutions - either by deprecating "niche" features (as we once did with
>>> various 'contrib' projects) or by recruiting new committers who have
>>> interest in maintaining that code area. Lifting review restrictions would
>>> allow us to sweep bus-factor issues under the rug.
>>> That said, I think CTR can be a valuable tool for "move fast on new
>>> experimentation" type projects -- I've participated in several feature
>>> branches in HDFS where we operated on CTR on the branch. The assumption
>> was
>>> that, prior to merging the branch into trunk, all of the patches (or a
>>> consolidated mega-patch) would be thoroughly reviewed by several other
>>> committers. I found this to work very well during early development,
>> since
>>> I could hack on things and even commit pieces of code that had known
>>> issues/TODOs. For trickier patches on the CTR branch, I still tended to
>>> ping experienced contributors for their opinions and feedback before
>>> committing.
>>> Perhaps such a hybrid policy would work well in the context of HBase
>>> feature development as well?
>>> -Todd
>>> On Wed, Jul 29, 2015 at 11:27 AM, Andrew Purtell <apurtell@apache.org>
>>> wrote:
>>>> Just thought of branch merge votes. Sorry for that omission. I think it
>>>> makes sense to keep those, but let's discuss that too in this context.
>>>> On Wed, Jul 29, 2015 at 11:26 AM, Andrew Purtell <apurtell@apache.org>
>>>> wrote:
>>>>> As Greg Stein said on a thread over at general@incubator
>>>>> Graduate Ignite from the Apache Incubator"):
>>>>> I always translate RTC as "we don't trust you, so somebody else must
>>>>> approve anything you do." To me, that is a lousy basis for creating a
>>>>> community. Trust and peer respect should be the basis, which implies
>>> CTR.
>>>>> I happen to agree with this sentiment and would like to propose
>>> switching
>>>>> our consensus on commit procedure from RTC to CTR. Concurrently, I
>>> would
>>>>> like to propose this consensus also include the following:
>>>>> - Checkins should pass precommit or the committer should explain on
>> the
>>>>> JIRA why precommit failures are in their best judgement not related
>>>>> - The PMC should be willing to, ultimately, revoke committership
>> should
>>>>> trust in any individual committer be discovered to be misplaced. I
>>> would
>>>>> expect such a last resort to be exceedingly rare and likely never to
>>>> happen
>>>>> because of course long before that we would be setting correct public
>>>>> examples in the first place and respectfully counseling well
>>> intentioned
>>>>> committers who might stray.
>>>>> Depending on how the conversation proceeds here I would like to
>> include
>>>>> dev@ in this conversation at the earliest opportunity as well.
>>>>> Thoughts?
>>>>> --
>>>>> Best regards,
>>>>>   - Andy
>>>>> Problems worthy of attack prove their worth by hitting back. - Piet
>>> Hein
>>>>> (via Tom White)
>>>> --
>>>> Best regards,
>>>>   - Andy
>>>> Problems worthy of attack prove their worth by hitting back. - Piet
>> Hein
>>>> (via Tom White)
>>> --
>>> Todd Lipcon
>>> Software Engineer, Cloudera
> -- 
> Sean

View raw message