we should have a betting pool on what the current coverage is. On Wed, Jun 20, 2018 at 7:54 PM Naveen Swamy <mnnav...@gmail.com> 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 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 > know > > the confidence of the system. This impact is one of the aspects I'd like > to > > assess in the next weeks. > > > > The initial goal is to give the reviewer an additional tool to assess the > > coverage of one's PR. One example could be the optimization of some > backend > > logic. The reviewer would then see if the changed codeis actually being > hit > > by any tests or if they are being missed entirely. I agree that just the > > percentage could be highly misleading and we should review that very > > clearly. > > > > For now, I'd only like to gather the data in the background. I would > follow > > up with a big thread on dev@ about my observations, my recommendations > and > > the possible options we have to use the data. We can then decide as > > community how exactly we would like to proceed and how much importance we > > give to the generated reports. > > > > Best regards, > > Marco > > > > > > > > On Wed, Jun 20, 2018 at 7:23 PM Tianqi Chen <tqc...@cs.washington.edu> > > wrote: > > > > > 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 APIs and automatically generated wrapper. > > > Sometimes the code-cov shows a negative impact on coverage while CI > > passes, > > > and the giving information was quite misleading if you just look at the > > PR > > > tabs > > > > > > I would still trust the reviewer's decision on the pull request > merging. > > > > > > Tianqi > > > > > > On Wed, Jun 20, 2018 at 7:14 PM, Marco de Abreu < > > > marco.g.ab...@googlemail.com.invalid> wrote: > > > > > > > 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 need to gather some data first, I'd like to request merging > > > > https://github.com/apache/incubator-mxnet/pull/11344. This will > enable > > > > publishing the coverage data to the service and have no impact on > your > > > PRs > > > > - it will just allow me to assess the quality of the service in the > > > > background while I come up with a full integration design. My initial > > > plan > > > > is to start with coverage across Python and C++ and then, with the > help > > > of > > > > our community, extend the report across all our supported languages. > > > > > > > > Does anybody object to having us gather this data in the background? > > > > > > > > Best regards, > > > > Marco > > > > > > > > > >
