I don't really agree that RTC means "we do not trust you". It just means that
changes should be peer reviewed, with which I heartily agree.CTR can work with
small group (for example in a branch). For a big project I think it will lead
to lower quality (and we already have issues with constantly failing tests -
part due to the infrastructure, but in part because they are flaky).
That said, I like the idea to leave this at discretion of the committer. In
that case we do not need the specific week time-line. For a small fix I think a
committer can just commit without review at all (null checks, etc). For larger
changes or features the committer should naturally request some review.
Not a fan of codifying too much details, it's better to trust the judgment of
the committer and state some general guidelines.
So what am I saying?
I think we can state that review is not _required_. Period. Then we could state
that committers should use good judgment as to when to request feedback.
-- Lars
From: Andrew Purtell <andrew.purt...@gmail.com>
To: "dev@hbase.apache.org" <dev@hbase.apache.org>
Cc: "priv...@hbase.apache.org" <priv...@hbase.apache.org>
Sent: Thursday, July 30, 2015 6:15 PM
Subject: Re: [DISCUSSION] Switching from RTC to CTR
I appreciate very much the earlier feedback about switching from RTC to
CTR. It helped me think about the essential thing I was after.
I'm thinking of making a formal proposal to adopt this, with a VOTE:
> After posting a patch to JIRA, after one week if there is no review or
veto, a committer can commit their own work.
It's important we discuss this and have a vote because the default
Foundation decision making process (
http://www.apache.org/foundation/voting.html) does not allow what would
amount to lazy consensus when RTC is in effect. Should my proposal pass, we
would arrive at a hybrid policy that is identical to the default Foundation
one *until* one week has elapsed after a code change is proposed. Then, for
a committer, for that one code change, they would be able to operate using
CTR. I think the HBase PMC is empowered to set this kind of policy for our
own project at our option. If you feel I am mistaken about that, please
speak up. Should the vote pass I will run it by board@ for review to be
sure.
We'd document this in the book:
https://hbase.apache.org/book.html#_decisions
Also, looking at https://hbase.apache.org/book.html#_decisions, I don't
think the "patch +1 policy" should remain because the trial OWNERS concept
hasn't worked out, IMHO. The OWNERS concept requires a set of constantly
present and engaged owners, a resource demand that's hard to square with
the volunteer nature of our community. The amount of time any committer or
PMC member has on this project is highly variable day to day and week to
week. I'm also thinking of calling a VOTE to significantly revise or
strike this section.
Both of these things have a common root: Volunteer time is a very precious
commodity. Our community's supply of volunteer time fluctuates. I would
like to see committers be able to make progress with their own work even in
periods when volunteer time is in very short supply, or when they are
working on niche concerns that simply do not draw sufficient interest from
other committers. (This is different from work that people think isn't
appropriate - in that case ignoring it so it will go away would no longer
be an option, a veto would be required if you want to stop something.)
On Wed, Jul 29, 2015 at 3:56 PM, Andrew Purtell <andrew.purt...@gmail.com>
wrote:
> Had this thought after getting back on the road. As an alternative to any
> sweeping change we could do one incremental but very significant thing that
> acknowledges our status as trusted and busy peers: After posting a patch to
> JIRA, after one week if there is no review or veto, a committer can commit
> their own work.
>
>
> > On Jul 29, 2015, at 2:20 PM, Mikhail Antonov <olorinb...@gmail.com>
> wrote:
> >
> > 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
>
