hawq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lei Chang <chang.lei...@gmail.com>
Subject Re: Staying with RTC (review-then-commit) rule
Date Wed, 30 Sep 2015 06:34:09 GMT
The original process (link
<https://mail.google.com/mail/u/0/(https://cwiki.apache.org/confluence/display/HAWQ/Contributing+to+HAWQ)>)
that
are agreed from pivotal engineering team just before the os drop says code
reviews. In the process, pull request is needed. The argument here comes
from how many votes needed before commit.

IMO, for trivial commits, committers can decide. It saves a lot of time.
And for non-trivial commits, code review are needed. 1 or 2 votes does not
make a lot of difference here. This process is quite similar to what we had
done past for hawq.

Changing to something more rigid and waste committers' time looks not a
good reason for change. If some obviously sign says we need more rigid
rules, then we can change it.

CI perspective, it does not mean pivotal internal. CI should work on Apache
side too. Roman has started some Jenkins projects, and we will make it work
soon and more tests will be opened step by step.

Cheers
Lei


On Wed, Sep 30, 2015 at 2:54 AM, Caleb Welton <cwelton@pivotal.io> wrote:

> To a degree repeating the conversation is healthy.  Projects are different
> and there are trade offs between different mechanisms.  We should read
> through the other projects conversations, understand why they made the
> choices they did, listen to the advice of our mentors and come to our own
> conclusions via discussion.
>
> I see the benefit of the suggestion to "start with RTC and then relax" as
> something that deliberately forces us as a community look at the Apache
> voting process, show that we understand it and can follow it, and
> understand the impact.  Once that goal has been achieved then relaxing to
> CTR streamlines things while still working with best practices.  Basically
> training wheels as we are working through the initial growing pains of
> incubation.
>
> One additional orthogonal comment.  You mention "Any change that touches
> code or test, whether reviewed or not, must have full CI  <
> http://204.14.53.153/overview.html>test suite pass."
>
> Wearing my Apache hat:
> If that's what Pivotal wants to do that's fine and up to them, but as an
> Apache project we cannot institutionalize and require a commit process that
> relies on closed test frameworks not available to the community.
>
> Still hoping we can get more committers to weigh in...
>
> > On Sep 29, 2015, at 10:27 PM, CHANG Lei <chang.lei.cn@gmail.com> wrote:
> >
> > I think we are going to repeat the discussions other guys have discussed.
> >
> > Personally, I think, RTC is rigid and restrictive. Different cases should
> > be handled differently.
> >
> > For small changes, RTC is somewhat too heavy for committers.
> >
> > I think it is a good choice to let committers decide which one need
> review
> > and which one does not.
> >
> > One example is about deleting the p4 ignore file, to send an email to
> > solicit review and waste several developers time do not worth it.
> >
> > Like the conclusion ignite project get.
> >
> >   - It is up to a committer to decide whether a change should need a
> >   review or not.
> >   - Any change that touches code or test, whether reviewed or not, must
> >   have full CI  <http://204.14.53.153/overview.html>test suite pass.
> >   - As a guideline, trivial changes, like updates to documentation or
> >   tests, etc. should not need a review prior to committing, and often
> can be
> >   made directly in the master branch.
> >   - *It is strongly recommended to have non-trivial changes reviewed by
> at
> >   least one other committer.*
> >
> >
> > If we get everyone's feedback, we might get the same consensus.
> >
> > Cheers
> > Lei
> >
> >
> >> On Tue, Sep 29, 2015 at 8:48 PM, Caleb Welton <cwelton@pivotal.io>
> wrote:
> >>
> >> Konstantin, thanks for the references, it's helpful seeing what other
> >> projects have done and the discussions they have had.
> >>
> >> Right now I'm inclined to follow Roman's advice of starting with RTC at
> >> least until we've gotten into the swing of things Apache style.  This
> also
> >> seems in-line with the contribution guidelines that Lei sketched out [1]
> >> which seems to assume a RTC model.
> >>
> >> The LazyConsensus model seems like a good addition to a pure RTC.  The
> >> voting guidelines that you posted seem to indicate 3 +1's are typical.
> >> From a hallway conversation with Roman it sounded like 1 or 2 are
> equally
> >> common.
> >>
> >> So... I propose:
> >>  - Use RTC and revisit if we feel this is too much overhead
> >>  - Start with 2 +1's defining consensus
> >>  - Allow LazyConsensus
> >>  - Update the contribution guidelines accordingly
> >>
> >> Agree/Disagree?  Hopefully we can get some more people to weigh in.
> >>
> >> Caleb
> >>
> >> [1]
> >>
> >>
> https://cwiki.apache.org/confluence/display/HAWQ/Contributing+to+HAWQ#ContributingtoHAWQ-Codereview
> >>
> >>
> >> On Tue, Sep 29, 2015 at 12:12 PM, Konstantin Boudnik <cos@apache.org>
> >> wrote:
> >>
> >>>> On Mon, Sep 28, 2015 at 11:06PM, CHANG Lei wrote:
> >>>> CTR (Commit then Review) or RTC (Review then commit) is a process we
> >> need
> >>>> to finalize soon. Actually, most of the hawq code developed before
> >>> follows
> >>>> CTR.
> >>>>
> >>>> And I just found some other commits on the branch (that does not get
2
> >>> +1s,
> >>>> looks from Caleb :-)
> >>>>
> >>>> So IMO, to get this finalized might be a good practice for an apache
> >>> voting
> >>>> process on Procedural item (
> >> http://www.apache.org/foundation/voting.html
> >>> ).
> >>>
> >>> While using voting to rubber-stamp a procedural vote is a legit use of
> >> the
> >>> voting process, let's only use it after the consensus [1] is reached.
> >>> Voting
> >>> isn't a replacement nor a right mechanism to establish policies or
> >>> practices
> >>> within the community. A couple of good examples of gaining the
> consensus
> >>> through the community relevant to the topic are [2] and [3]
> >>>
> >>> Cos
> >>>
> >>> [1] https://www.apache.org/foundation/voting.html#LazyConsensus
> >>> [2] http://bit.ly/1gLeArc
> >>> [3]
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/Jira-Process-tp1816p1831.html
> >>>
> >>>> Most committers do not know about the process.
> >>>>
> >>>> Cheers
> >>>> Lei
> >>>>
> >>>>
> >>>>> On Mon, Sep 28, 2015 at 8:45 PM, Caleb Welton <cwelton@pivotal.io>
> >>>> wrote:
> >>>>
> >>>>> Konstantin, I don't think all the developers are aware of the
> >>> differences
> >>>>> between CTR and RTC.  Would you care to elaborate on the contribution
> >>>>> protocols?
> >>>>>
> >>>>> Thanks!
> >>>>>
> >>>>>
> >>>>> On Mon, Sep 28, 2015 at 2:29 PM, Konstantin Boudnik <cos@apache.org>
> >>>>> wrote:
> >>>>>
> >>>>>> I am the big advocate of CTR model and has been pushing it in
a
> >>> number of
> >>>>>> the
> >>>>>> projects before. However, for the very early ones like Hawq
right
> >>> now,
> >>>>> RTC
> >>>>>> might make more sense: it takes time to learn how to add new
trusty
> >>>>> people
> >>>>>> to
> >>>>>> the community; also for new committers it helps to learn the
code
> >>> faster
> >>>>> by
> >>>>>> doing the mandatory reviews. But before you know it, I will
be
> >> doing
> >>>>> rounds
> >>>>>> here pushing for CTR ;)
> >>>>>>
> >>>>>> Cos
> >>>>>>
> >>>>>>> On Mon, Sep 28, 2015 at 03:11PM, Roman Shaposhnik wrote:
> >>>>>>> Hi!
> >>>>>>>
> >>>>>>> just wanted to say what I said on HAWQ-7, but
> >>>>>>> since not all of us are watching that JIRA, I think
> >>>>>>> it would be useful to repeat it here.
> >>>>>>>
> >>>>>>> While there's nothing wrong with lazy consensus,
> >>>>>>> but initially I'd suggest staying with RTC model.
> >>>>>>> Thus +1 from somebody else should be explicitly
> >>>>>>> required.
> >>>>>>>
> >>>>>>> I don't think there's any disagreement, but I just
> >>>>>>> wanted to have it explicitly mentioned.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Roman.
> >>
>

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