mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Xie <j...@apache.org>
Subject Re: Reverting pull request
Date Fri, 15 Jun 2018 22:07:42 GMT
The way I see it:

1) you are just complaining and have never written code that fixes flaky tests
2) you are actively introducing bugs to the CI that causes it to fail in ways unrelated to
any tests

Eric

On 2018/06/15 22:03:29, Marco de Abreu <marco.g.abreu@googlemail.com.INVALID> wrote:

> CI doesn't fail for no reason but because some people prefer to push new
> features than to get our codebase actually stable. We currently have 51 [1]
> flaky tests and I have only seen a few people (thanks Sheng, Alex and
> Pedro) work on the problem. So instead of complaining, take part and help
> improving the situation.
> 
> The CCache/EFS failure lasted for 12 hours and was an error - these things
> happen when you run a service. This is not a blame-game.
> 
> -Marco
> 
> [1]:
> https://github.com/apache/incubator-mxnet/issues?q=is%3Aopen+is%3Aissue+label%3AFlaky
> 
> On Fri, Jun 15, 2018 at 2:55 PM Eric Xie <jxie@apache.org> wrote:
> 
> > Hi Marco de Abreu,
> >
> > CI has been totally broken recently. It randomly fails for no good reason
> > more often than it passes. For example the ccache/efs failure has been
> > really annoying.
> >
> > Looks like there has been many changes to Jenkins and Docker lately. Do
> > you think we should revert all of the recent changes to get a stable CI or
> > do you think someone should find a fix for the bugs?
> >
> > Thanks,
> > Eric
> >
> > On 2018/06/15 21:21:50, 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
View raw message