mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Naveen Swamy <mnnav...@gmail.com>
Subject Re: Protected master needs to be turned off
Date Mon, 20 Nov 2017 18:17:40 GMT
-1
for the reasons, Gautam and Marco have pointed out.

On Sun, Nov 19, 2017 at 4:07 PM, Gautam <gautamnitc@gmail.com> wrote:

> -1
>
> Please see inline.
>
> On Nov 19, 2017 12:51 PM, "Eric Xie" <jxie@apache.org> wrote:
>
> Hi all,
> I'm starting this thread to vote on turning off protected master. The
> reasons are:
>
> 1. Since we turned on protected master pending PRs has grown from 40 to 80.
> It is severely slowing down development.
>
>
>      Turning protection off, will give you ability to merge the code which
> has build failure. How and more importantly who will be best judge to
> figure out what's is wrong ? If CI is a problem then we have people from
> infra team, who are sort of 'on call" day and night trying to fix.
> Including me trying to fix the slave at 2 am in night. I have seen
> commiters sending PRs and if it fails without even looking at the
> reason,which could be just a temporary error, they reach out to infra team
> immediately. So I don't agree that we should turn off the protected branch
> just for the sake of speed. We should care more on quality than speed.
>
>
> 2. Committers, not CI, are ultimately responsible for the code they merge.
> You should only override the CI when you are very confident that CI is the
> problem, not your code. If it turns out you are wrong, you should fix it
> ASAP. This is the bare minimum requirement for all committers: BE
> RESPONSIBLE.
>
> How do we make sure that this in deed happens?
>
> I'm aware of the argument for using protected master: It make sure that
> master is stable.
>
> Well, master will be most stable if we stop adding any commits to it. But
> that's not what we want is it?
>
> No we are not saying don't add anything in master, we are just saying
> please don't add bad code to the master. And yes I have seen bad code has
> been merged to the master when protected branch was not enabled.
>
> Protected master hardly adds any stability. The faulty tests that breaks
> master at random got merged into master because they happened to succeed
> once.
>
> That's not true, it filter out one of the important aspect that at least
> when code was merged it completed the whole cycle of build and test. Sure
> flaky test we can track down.
>
> Thanks,
> Junyuan Xie
>

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