Just curious, I assume if this change is made, would it only apply to
master branch?
-Mikhail
On Wed, Jul 29, 2015 at 2:09 PM, Andrew Purtell
<andrew.purt...@gmail.com> wrote:
> @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.bus...@gmail.com> wrote:
>>
>> I'd also favor having this discussion on dev@.
>>
>>> On Wed, Jul 29, 2015 at 2:29 PM, Gary Helmling <ghelml...@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 <t...@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 <apurt...@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 <apurt...@apache.org>
>>>>> wrote:
>>>>>
>>>>>> As Greg Stein said on a thread over at general@incubator
>>>> ("[DISCUSSION]
>>>>>> 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
--
Thanks,
Michael Antonov
