Re: Protected master needs to be turned off

Agree on both Mu and Tianqi’s points. I think this would align well with the “nightly tests” narrative. We can disable CI for trivial changes with a comment like (“skip ci”) comment on the ghprb Jenkins plugin, and trust the nightly to catch any problems introduced by trivial patches in an

Re: Protected master needs to be turned off

We plan to add the possibility to trigger or stop a build by adding a comment in the pull request - this will be added after we switched to the new CI. Am 01.12.2017 7:25 nachm. schrieb "Mu Li" : > That's how it works before, committers can stop a CI test if it's a simple >

Re: Protected master needs to be turned off

That's how it works before, committers can stop a CI test if it's a simple fix, such as typo. But with apache's jenkins server, it's nontrivial to request the permission to stop/start a CI test. I had a thought about to let the CI be smart enough to only run the tests related to the code

Re: Protected master needs to be turned off

I think we are still using CI to catch bugs. And we should take caution when merging something that CI did not catch up due to the response time. This do not contradict what comitter can do with their best judgement. For example, I would normally switch CI off and merge simple typo fixes. If the

Re: Protected master needs to be turned off

Totally agree that CI is useful. Actually, I wrote the jenkinsfile and setup the jenkins server before moving to apache server. I just mention that we cannot rely on the CI test. It currently covers operator unittests and regression test on several cnns. But the code coverage isn't great. If a PR

Re: Protected master needs to be turned off

CI catches problems all the time. I don't think many of us can afford to build all the flavors and architectures in their laptops or workstations, so we have to rely on CI to catch all kinds of errors from compilation errors to bugs plus regressions, specially in a project which has so many build

Re: Protected master needs to be turned off

Sounds like a good way to me. By the way, the setup for the new CI is ready and we’re already verifying the master branch. We still need to do some clean up and await the Amazon-AppSec review. On Thu, Nov 30, 2017 at 8:55 PM, Mu Li wrote: > I barely remember CI caught bugs

Re: Protected master needs to be turned off

I barely remember CI caught bugs except for lint, but it definitely slows down the code merge. I understand the general concerns for removing master protection. So I propose to use the dev branch to merge changes until the CI is stable. And make the nightly build build the dev branch instead

Re: Protected master needs to be turned off

Since committers voted for +1. We consider this vote passed. Thanks, Eric On 2017-11-19 12:51, "Eric Xie" 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

Re: Protected master needs to be turned off

-0.5 (non-binding). I would propose that if there are pieces of CI that are slowing down development (and I think we can all agree that there are) that we should strip out these problematic CI pieces and open issues for them. We can then assign issues to people and evaluate what should be done

Re: Protected master needs to be turned off

Small update regarding the status of the new CI: We've been able to get ubuntu builds and tests up and running. I’ve received requests to provide exact times for all important stages of the CI. As the first runs have been executed successfully, I’ll provide you with some data. This will give you

Re: Protected master needs to be turned off

+1 until new CI is implemented. Tianqi On Mon, Nov 20, 2017 at 11:11 AM, Eric Xie wrote: > A lot of people seems to be confused, so let's clarify the separation of > roles/responsibilities: > > 1. The committers that merge code are responsible for code quality and > tests

Re: Protected master needs to be turned off

-1 (non binding) While I agree that the protected master is slowing development but it is also helping identify problematic tests. If we don't do this now, the same issues will exist on the new CI. On Sun, Nov 19, 2017 at 1:51 PM, Marco de Abreu < marco.g.ab...@googlemail.com> wrote: > Hello, >

Re: Protected master needs to be turned off

-1 for the reasons, Gautam and Marco have pointed out. On Sun, Nov 19, 2017 at 4:07 PM, Gautam wrote: > -1 > > Please see inline. > > On Nov 19, 2017 12:51 PM, "Eric Xie" wrote: > > Hi all, > I'm starting this thread to vote on turning off protected

Re: Protected master needs to be turned off

-1 Please see inline. On Nov 19, 2017 12:51 PM, "Eric Xie" 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.

Re: Protected master needs to be turned off

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

Re: Protected master needs to be turned off

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

Re: Protected master needs to be turned off

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

Re: Protected master needs to be turned off

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

Re: Protected master needs to be turned off

+1 On Sun, Nov 19, 2017 at 12:52 PM Zha, Sheng wrote: > +1 > > Best regards, > -sz > > On 11/19/17, 12:51 PM, "Eric Xie" wrote: > > Hi all, > I'm starting this thread to vote on turning off protected master. The > reasons are: > > 1. Since we