geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Helena Bales <hba...@pivotal.io>
Subject Re: [DISCUSS] require reviews before merging a PR
Date Fri, 31 May 2019 18:00:10 GMT
Thanks for the filter, Jinmei!

+1 to Naba and Bruce.

I will add that I think we should have a formal policy of getting *a*
review (and more for large changes), and that PRs that are merged without
one should include (in the PR) a comment providing a justification for this
merge, so that committers can review the reasoning later on if needed. This
has the benefits of being low impact on our current workflow, but also
increasing transparency, accountability, and thoughtfulness around the
proposed changes and their impact.

On Fri, May 31, 2019 at 10:42 AM Jinmei Liao <jiliao@pivotal.io> wrote:

> I was told that screenshot that I sent earlier got filtered out by the dev
> list. Basically, the filter puts "notifications@github.com" in the "From"
> section, and put "review_requested@noreply.github.com" in the "Doesn't
> have" section of the form.
>
> On Fri, May 31, 2019 at 10:36 AM Anthony Baker <abaker@pivotal.io> wrote:
>
> >
> >
> > > On May 31, 2019, at 10:01 AM, Owen Nichols <onichols@pivotal.io>
> wrote:
> > >
> > > We chose to make Geode an Apache open source project for a reason.  If
> > we no longer wish to embrace The Apache Way <
> > https://www.apache.org/theapacheway/index.html>, perhaps we should
> > reconsider.
> >
> > I strongly disagree with the assertion that we are not following the
> > Apache Way because we aren’t doing RTC.  Please take a look around other
> > ASF communities and compare that to our approach.  I think you’ll see a
> lot
> > of similarities in the way we review GitHub pull requests.
> >
> > >
> > > If we believe that reviewing each other’s code changes is an onerous
> > burden of no value, we should question why.   The long-term success of
> > Geode depends on sharing of knowledge, not “cowboy coders”.  3 reviews
> > means now 3 other people are more familiar with that part of the code…
> >
> > Yes of course:  community >> code.  Can you point me to cases of “cowboy
> > coding” in Geode?  I’m not seeing it but happy to be convinced otherwise.
> >
> > >
> > > If apathy is our thing, Apache does allows for “lazy consensus”, but
> you
> > have to declare that you will be using it, e.g. “This PR fixes
> > GEODE-123456; if no-one objects within three days, I'll assume lazy
> > consensus and merge it.”
> >
> > IMO lazy consensus does not imply apathy.
> >
> >
> >
> > Anthony
> >
> >
>
> --
> Cheers
>
> Jinmei
>

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