Committer right is a privilege. If someone keeps merging untested code and refuses to fix the problems they have caused then he or she shouldn't be a committer.
Using protected master doesn't guarantee that code is well tested. You can disable the test and merge code instead of actually fixing the bug. Using protected master actually encourages this behavior, since people are focused on getting tests to pass. It doesn't do much good towards improving code quality. As far as I know, no other team managing large code bases uses protected master without the ability to override. Thanks, Junyuan Xie On 2017-11-19 13:51, Marco de Abreu <[email protected]> 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" <[email protected]>: > > > Revised: > > > > > > +1 at least until new CI is implemented. Then reevaluate. > > > > On Sun, Nov 19, 2017 at 1:07 PM Chris Olivier <[email protected]> > > wrote: > > > > > +1 > > > > > > > > > On Sun, Nov 19, 2017 at 12:52 PM Zha, Sheng <[email protected]> wrote: > > > > > >> +1 > > >> > > >> Best regards, > > >> -sz > > >> > > >> On 11/19/17, 12:51 PM, "Eric Xie" <[email protected]> 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 > > >> > > >> > > >> > > >> > > >
