mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mu Li <muli....@gmail.com>
Subject Re: Reverting pull request
Date Fri, 15 Jun 2018 22:01:59 GMT
send to

*dev*-*un*subscribe@mxnet.incubator.apache.org


On Fri, Jun 15, 2018 at 2:59 PM, Chris Olivier <cjolivier01@gmail.com>
wrote:

> does anyone know how to unsubscribe from this list?
>
>
> On Fri, Jun 15, 2018 at 2:56 PM Haibin Lin <haibin.lin.aws@gmail.com>
> wrote:
>
> > Why revert the PR when we know there's a fix?
> > If we keep going backwards like this, no progress can be made.
> >
> > On Fri, Jun 15, 2018 at 2:37 PM, Mu Li <muli.cmu@gmail.com> wrote:
> >
> > > Agree that major changes need more extensive reviews. But we cannot
> > ignore
> > > that both reviews and CI cannot catch all bugs. Reverting each PR after
> > > finding a bug should be the last ways, before it, we should try to fix
> it
> > > first.
> > >
> > > As for the breaking change, I see it differently. It breaks a not
> > > recommended usage of the API from an unmaintained tutorial, I don't
> think
> > > adding more reviewers will help it.
> > >
> > > Besides, I'm less sure if we can find enough reviewers to provide
> useful
> > > feedbacks for major changes.
> > >
> > > On Fri, Jun 15, 2018 at 2:21 PM, Marco de Abreu <
> > > marco.g.abreu@googlemail.com.invalid> wrote:
> > >
> > > > We revert a PR because it should not have been merged in the first
> > place.
> > > > So far, I have been ignoring the fact that our committers are
> > constantly
> > > > breaking our own rules (which we expect contributors to follow). But
> > > since
> > > > this caused an impact twice (1.2 breaking change about model
> > > import/export
> > > > as well as this regression), I'm now being more strict and enforcing
> > > them.
> > > >
> > > > I could've also made a script that prevents any PR from being
> > > self-merged,
> > > > but I thought our committers are responsible enough to follow our own
> > > rules
> > > > without systems actually enforcing them. I won't waste my time
> working
> > on
> > > > that script, but from now on I will revert every single PR (except
> > > > emergency cases) that has been self-merged without approval.
> > > >
> > > > -Marco
> > > >
> > > > On Fri, Jun 15, 2018 at 2:15 PM Mu Li <muli.cmu@gmail.com> wrote:
> > > >
> > > > > Why reverting instead of fixing the bugs? Static memory aims to
> > reduce
> > > > > memory allocation, it's a key feature to bridge the perf gap
> between
> > > > gluon
> > > > > and symbol.
> > > > >
> > > > > On Fri, Jun 15, 2018 at 2:06 PM, Marco de Abreu <
> > > > > marco.g.abreu@googlemail.com.invalid> wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > I'm reverting https://github.com/apache/
> incubator-mxnet/pull/10817
> > > as
> > > > of
> > > > > > https://github.com/apache/incubator-mxnet/pull/11311 due to
> > > > regressions
> > > > > > described in
> > https://github.com/apache/incubator-mxnet/issues/11171
> > > > and
> > > > > > https://github.com/apache/incubator-mxnet/pull/10817.
> > > > > >
> > > > > > The pull request has been self-merged without proper review
and
> > > > > introduced
> > > > > > regressions. Committers should act as role models in this project
> > and
> > > > > > adhere to software engineer best practices.
> > > > > >
> > > > > > Best regards,
> > > > > > Marco
> > > > > >
> > > > >
> > > >
> > >
> >
>

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