Re: Protected master needs to be turned off

[Pedro Larroy <pe...@gmail.com>]

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 flavors.

I have had this experience in big projects several times and I can
tell you it's always the same.

So from extensive software development experience I write that we will
be able to develop and merge much faster once we have a reliable CI
running in short cycles, any other approach or shortcuts is just
accumulating technical debt for the future that somebody will have to
cleanup and will slow down development. Is better to have a CI with a
reduced scope working reliably than bypassing CI.

This is irrespective of using dev to merge or unprotected master.

We can't afford to have increased warnings, bugs creeping into the
codebase going unnoticed, build system problems, performance
regressions, etc. And we have to rely on a solid CI for this. If we
are not ready for this, we should halt feature development or at least
merging new features until we have a stable codebase and build system.

Re: Protected master needs to be turned off

[Pedro Larroy <pe...@gmail.com>]

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 aggregated manner.

Pedro.

On Fri, Dec 1, 2017 at 6:49 PM, Tianqi Chen <tq...@cs.washington.edu> wrote:
> 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
> fix merged causes problem, you still get CI to report it maybe a few hours
> later.
>
> The bottom line is that comitter get these information as feedbacks and
> they use their best judgement when do so. Most of the time when unsure, I
> simply wait for CI to finish
>
> Tianqi
>
>
> On Fri, Dec 1, 2017 at 9:41 AM Mu Li <mu...@gmail.com> wrote:
>
>> 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 touches the core system, the best practice today is still code
>> reviewing. Otherwise, such as a PR is mainly about examples, the CI often
>> doesn't help so we just waste machine times.
>>
>> I think checking the exact code coverage is on the roadmap, but I don't
>> know if we have any progress on it.
>>
>> On Fri, Dec 1, 2017 at 6:19 AM, Pedro Larroy <pedro.larroy.lists@gmail.com
>> >
>> wrote:
>>
>> > 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 flavors.
>> >
>> > I have had this experience in big projects several times and I can
>> > tell you it's always the same.
>> >
>> > So from extensive software development experience I write that we will
>> > be able to develop and merge much faster once we have a reliable CI
>> > running in short cycles, any other approach or shortcuts is just
>> > accumulating technical debt for the future that somebody will have to
>> > cleanup and will slow down development. Is better to have a CI with a
>> > reduced scope working reliably than bypassing CI.
>> >
>> > This is irrespective of using dev to merge or unprotected master.
>> >
>> > We can't afford to have increased warnings, bugs creeping into the
>> > codebase going unnoticed, build system problems, performance
>> > regressions, etc. And we have to rely on a solid CI for this. If we
>> > are not ready for this, we should halt feature development or at least
>> > merging new features until we have a stable codebase and build system.
>> >
>>

Re: Protected master needs to be turned off

[Marco de Abreu <ma...@googlemail.com>]

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" <mu...@gmail.com>:

> 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 changes. But I felt it's easier to have committers to
> do it manually. Given the current situation, however, it's probably worth
> spending times to improve the CI instead of requesting changes to the
> repo/CI server.
>
> On Fri, Dec 1, 2017 at 9:49 AM, Tianqi Chen <tq...@cs.washington.edu>
> wrote:
>
> > 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
> > fix merged causes problem, you still get CI to report it maybe a few
> hours
> > later.
> >
> > The bottom line is that comitter get these information as feedbacks and
> > they use their best judgement when do so. Most of the time when unsure, I
> > simply wait for CI to finish
> >
> > Tianqi
> >
> >
> > On Fri, Dec 1, 2017 at 9:41 AM Mu Li <mu...@gmail.com> wrote:
> >
> > > 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 touches the core system, the best practice today is still code
> > > reviewing. Otherwise, such as a PR is mainly about examples, the CI
> often
> > > doesn't help so we just waste machine times.
> > >
> > > I think checking the exact code coverage is on the roadmap, but I don't
> > > know if we have any progress on it.
> > >
> > > On Fri, Dec 1, 2017 at 6:19 AM, Pedro Larroy <
> > pedro.larroy.lists@gmail.com
> > > >
> > > wrote:
> > >
> > > > 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 flavors.
> > > >
> > > > I have had this experience in big projects several times and I can
> > > > tell you it's always the same.
> > > >
> > > > So from extensive software development experience I write that we
> will
> > > > be able to develop and merge much faster once we have a reliable CI
> > > > running in short cycles, any other approach or shortcuts is just
> > > > accumulating technical debt for the future that somebody will have to
> > > > cleanup and will slow down development. Is better to have a CI with a
> > > > reduced scope working reliably than bypassing CI.
> > > >
> > > > This is irrespective of using dev to merge or unprotected master.
> > > >
> > > > We can't afford to have increased warnings, bugs creeping into the
> > > > codebase going unnoticed, build system problems, performance
> > > > regressions, etc. And we have to rely on a solid CI for this. If we
> > > > are not ready for this, we should halt feature development or at
> least
> > > > merging new features until we have a stable codebase and build
> system.
> > > >
> > >
> >
>

Re: Protected master needs to be turned off

[Mu Li <mu...@gmail.com>]

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 changes. But I felt it's easier to have committers to
do it manually. Given the current situation, however, it's probably worth
spending times to improve the CI instead of requesting changes to the
repo/CI server.

On Fri, Dec 1, 2017 at 9:49 AM, Tianqi Chen <tq...@cs.washington.edu>
wrote:

> 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
> fix merged causes problem, you still get CI to report it maybe a few hours
> later.
>
> The bottom line is that comitter get these information as feedbacks and
> they use their best judgement when do so. Most of the time when unsure, I
> simply wait for CI to finish
>
> Tianqi
>
>
> On Fri, Dec 1, 2017 at 9:41 AM Mu Li <mu...@gmail.com> wrote:
>
> > 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 touches the core system, the best practice today is still code
> > reviewing. Otherwise, such as a PR is mainly about examples, the CI often
> > doesn't help so we just waste machine times.
> >
> > I think checking the exact code coverage is on the roadmap, but I don't
> > know if we have any progress on it.
> >
> > On Fri, Dec 1, 2017 at 6:19 AM, Pedro Larroy <
> pedro.larroy.lists@gmail.com
> > >
> > wrote:
> >
> > > 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 flavors.
> > >
> > > I have had this experience in big projects several times and I can
> > > tell you it's always the same.
> > >
> > > So from extensive software development experience I write that we will
> > > be able to develop and merge much faster once we have a reliable CI
> > > running in short cycles, any other approach or shortcuts is just
> > > accumulating technical debt for the future that somebody will have to
> > > cleanup and will slow down development. Is better to have a CI with a
> > > reduced scope working reliably than bypassing CI.
> > >
> > > This is irrespective of using dev to merge or unprotected master.
> > >
> > > We can't afford to have increased warnings, bugs creeping into the
> > > codebase going unnoticed, build system problems, performance
> > > regressions, etc. And we have to rely on a solid CI for this. If we
> > > are not ready for this, we should halt feature development or at least
> > > merging new features until we have a stable codebase and build system.
> > >
> >
>

Re: Protected master needs to be turned off

[Tianqi Chen <tq...@cs.washington.edu>]

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
fix merged causes problem, you still get CI to report it maybe a few hours
later.

The bottom line is that comitter get these information as feedbacks and
they use their best judgement when do so. Most of the time when unsure, I
simply wait for CI to finish

Tianqi


On Fri, Dec 1, 2017 at 9:41 AM Mu Li <mu...@gmail.com> wrote:

> 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 touches the core system, the best practice today is still code
> reviewing. Otherwise, such as a PR is mainly about examples, the CI often
> doesn't help so we just waste machine times.
>
> I think checking the exact code coverage is on the roadmap, but I don't
> know if we have any progress on it.
>
> On Fri, Dec 1, 2017 at 6:19 AM, Pedro Larroy <pedro.larroy.lists@gmail.com
> >
> wrote:
>
> > 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 flavors.
> >
> > I have had this experience in big projects several times and I can
> > tell you it's always the same.
> >
> > So from extensive software development experience I write that we will
> > be able to develop and merge much faster once we have a reliable CI
> > running in short cycles, any other approach or shortcuts is just
> > accumulating technical debt for the future that somebody will have to
> > cleanup and will slow down development. Is better to have a CI with a
> > reduced scope working reliably than bypassing CI.
> >
> > This is irrespective of using dev to merge or unprotected master.
> >
> > We can't afford to have increased warnings, bugs creeping into the
> > codebase going unnoticed, build system problems, performance
> > regressions, etc. And we have to rely on a solid CI for this. If we
> > are not ready for this, we should halt feature development or at least
> > merging new features until we have a stable codebase and build system.
> >
>

Re: Protected master needs to be turned off

[Mu Li <mu...@gmail.com>]

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 touches the core system, the best practice today is still code
reviewing. Otherwise, such as a PR is mainly about examples, the CI often
doesn't help so we just waste machine times.

I think checking the exact code coverage is on the roadmap, but I don't
know if we have any progress on it.

On Fri, Dec 1, 2017 at 6:19 AM, Pedro Larroy <pe...@gmail.com>
wrote:

> 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 flavors.
>
> I have had this experience in big projects several times and I can
> tell you it's always the same.
>
> So from extensive software development experience I write that we will
> be able to develop and merge much faster once we have a reliable CI
> running in short cycles, any other approach or shortcuts is just
> accumulating technical debt for the future that somebody will have to
> cleanup and will slow down development. Is better to have a CI with a
> reduced scope working reliably than bypassing CI.
>
> This is irrespective of using dev to merge or unprotected master.
>
> We can't afford to have increased warnings, bugs creeping into the
> codebase going unnoticed, build system problems, performance
> regressions, etc. And we have to rely on a solid CI for this. If we
> are not ready for this, we should halt feature development or at least
> merging new features until we have a stable codebase and build system.
>