Re: [LAZY VOTE] Test coverage of PRs

2018-06-21 Thread kellen sunderland
I've got no objections. Appreciate you adding coverage support. On Thu, Jun 21, 2018, 10:06 PM Marco de Abreu wrote: > Tianqi and Kellen, do you mind if we move ahead and merge > https://github.com/apache/incubator-mxnet/pull/11344 now? > > -Marco > > On Thu, Jun 21, 2018 at 4:50 PM Yasser Zama

Re: [LAZY VOTE] Test coverage of PRs

2018-06-21 Thread Marco de Abreu
Tianqi and Kellen, do you mind if we move ahead and merge https://github.com/apache/incubator-mxnet/pull/11344 now? -Marco On Thu, Jun 21, 2018 at 4:50 PM Yasser Zamani wrote: > +1 but just as an MXNet lover newbie :) > > We at Apache Struts, already use "coveralls" and I personally like it a >

Re: [LAZY VOTE] Test coverage of PRs

2018-06-21 Thread Yasser Zamani
+1 but just as an MXNet lover newbie :) We at Apache Struts, already use "coveralls" and I personally like it a lot even with some limitations with it. It doesn't allow us to merge a PR with a not tested code. It reports us where are those not covered new codes. Regards. On 6/21/2018 7:13 PM, Ma

Re: [LAZY VOTE] Test coverage of PRs

2018-06-21 Thread Marco de Abreu
To avoid any confusion, I have disable the PR comments at https://github.com/apache/incubator-mxnet/pull/11344/files#diff-1f7b4b85ed8525c5239f741431a72872R25. Thus, the data will only be recorded in the background is then retrievable at https://codecov.io/gh/apache/incubator-mxnet/pulls. -Marco O

Re: [LAZY VOTE] Test coverage of PRs

2018-06-21 Thread Marco de Abreu
Hi Kellen, Thanks for your feedback. The same argument about a hosted service could be applied to GitHub - we could just use the git repository hosted under Apache Infra then and completely stick to JIRA. This service gives us the advantage of being free of charge, a mature project and having bui

Re: [LAZY VOTE] Test coverage of PRs

2018-06-21 Thread kellen sunderland
-0.1 (non-binding). Looks good, and I like the idea of adding coverage, however I have a few minor issues with this approach: First it seems to use a hosted service, which can disappear / be acquired / change terms at any time. In general I would try and avoid using services like this for Apache

Re: [LAZY VOTE] Test coverage of PRs

2018-06-20 Thread Chris Olivier
we should have a betting pool on what the current coverage is. On Wed, Jun 20, 2018 at 7:54 PM Naveen Swamy wrote: > +1 to collect data on coverage. > > On Wed, Jun 20, 2018 at 7:38 PM, Marco de Abreu < > marco.g.ab...@googlemail.com.invalid> wrote: > > > Hello, > > > > for now, this is only a t

Re: [LAZY VOTE] Test coverage of PRs

2018-06-20 Thread Naveen Swamy
+1 to collect data on coverage. On Wed, Jun 20, 2018 at 7:38 PM, Marco de Abreu < marco.g.ab...@googlemail.com.invalid> wrote: > Hello, > > for now, this is only a test for myself and a way to gather data in order > to give us the possibility to make a data-driven decision. So far, we have > not

Re: [LAZY VOTE] Test coverage of PRs

2018-06-20 Thread Marco de Abreu
Hello, for now, this is only a test for myself and a way to gather data in order to give us the possibility to make a data-driven decision. So far, we have not decided on the implications and which restrictions we will set as result for the produced metrics and I'd like to postpone that until we k

Re: [LAZY VOTE] Test coverage of PRs

2018-06-20 Thread Tianqi Chen
While I think test coverage is a nice information to have. I would object to using this as a metric to decide whether a PR should be merged. Code-cov act as a mere coverage of APIs, which is a useful aspect, it can be misleading in many cases, especially when such change involves cross-language API

[LAZY VOTE] Test coverage of PRs

2018-06-20 Thread Marco de Abreu
Hello, I'd like to introduce test coverage metrics of PRs using https://codecov.io/. This tool will aggregate coverage reports across multiple runs, platforms and technologies and gives contributors as well as reviewers a new tool that allows to improve the quality of a pull request. Since we nee