A possible compromise could be that we turn off protection for master branch for the Apache infra only with plans to turn protection back on when new CI is up.
Furthermore, if something is merged in prematurely that breaks master for some number of time (ie 3), then we turn protection back on immediately and without a vote (in order to enforce great care by those who would vote for protection to stay off). These failures could be broadcast on dev and why they shouldn’t count as one of the three can be disputed there. To turn protection off again would require another vote. On Sun, Nov 19, 2017 at 2:37 PM Zha, Sheng <zhash...@amazon.com> wrote: > My +1 vote stands. The vote is about what we should do right now, not > where we should be ideally in 3 months. I don’t think we can move forward > without disabling the branch protection, because current CI is not in any > state to base the merge-decisions on. For example, here’s why: > 1. Master branch protection is currently on. A change that breaks build > was still merged in on the 16th despite the protection. I wasn’t able to > merge the fix in yesterday for 8 hours because the CI tests fail. > 2. False negative rate is currently too high (see the red crosses in > https://github.com/apache/incubator-mxnet/pulls and > https://github.com/apache/incubator-mxnet/commits/master) > > People who are working on test infrastructure might say that it’s “enough > work to isolate and fix the current issues”, and I can certainly relate to > that. On the other hand, you too can probably empathize with the developers > who has “enough work to develop new features and write tests” without > having to deal with the broken CI. (note that my argument is on the CI > system, and flaky test cases are a separate issue). > > Regarding “doesn't that mean that our users and customers are also going > to face those issues”, I honestly don’t think the argument stands. Release > cycles and distribution channels, as well as the safety measures are there > exactly to isolate the problems to the development branch and protect the > users. If anything, turning on branch protection on release branches should > suffice. > > Finally, master branch protection being off doesn’t mean PRs can be merged > without being tested. Contributors own the code quality and are responsible > for the changes. Committers and reviewers are there to ensure that merged > changes are OK. > > Best regards, > -sz > > On 11/19/17, 1:51 PM, "Marco de Abreu" <marco.g.ab...@googlemail.com> > wrote: > > Hello, > > -1 (non binding) > > Who is going to be responsible for changes breaking tests and having > other > side effects after they have been merged? I'm afraid that this will > harm > further development. At the moment I'm the responsible person for > setting > up the new CI and so far have my results shown that not the CI itself > is > the problem but also the stability of our code as well as the tests > themselves. At the moment we are having big issues to get a stable CI > because MXNet seems to be relying on so specific architectures, > dependencies and other factors which I'm not even able to track down > that > this causes everything to be unstable. > > Just to point it out: If we encounter so many problems while setting > up a > CI system, doesn't that mean that our users and customers are also > going to > face those issues as soon as things are getting more complicated? This > is a > red flag in my opinion and I'm really looking forward to the usability > Sprint, but at the moment I'm afraid that an unprotected master will > make > the situation even worse. It's already enough work to isolate and fix > the > current issues, but if new untested changes get merged, this is going > to be > like fighting a wildfire with a bottle of water. > > So please revise your thoughts. If anybody is blocked by the protected > master, I would really appreciate it if they could approach me > personally > in order to help stabilising the current situation. Just feeding in > more > and more changes on one end while we're fixing issues on the other end > won't get us anywhere. > > Best regards, > Marco > > Am 19.11.2017 10:08 nachm. schrieb "Chris Olivier" < > cjolivie...@gmail.com>: > > > Revised: > > > > > > +1 at least until new CI is implemented. Then reevaluate. > > > > On Sun, Nov 19, 2017 at 1:07 PM Chris Olivier <cjolivie...@gmail.com > > > > wrote: > > > > > +1 > > > > > > > > > On Sun, Nov 19, 2017 at 12:52 PM Zha, Sheng <zhash...@amazon.com> > wrote: > > > > > >> +1 > > >> > > >> Best regards, > > >> -sz > > >> > > >> On 11/19/17, 12:51 PM, "Eric Xie" <j...@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. > > >> > > >> 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. > > >> > > >> 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? > > >> > > >> Protected master hardly adds any stability. The faulty tests > that > > >> breaks master at random got merged into master because they > happened to > > >> succeed once. > > >> > > >> Thanks, > > >> Junyuan Xie > > >> > > >> > > >> > > >> > > > > >
