+1 to collect data on coverage. On Wed, Jun 20, 2018 at 7:38 PM, Marco de Abreu < [email protected]> 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 <[email protected]> > 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 < > > [email protected]> 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 > > > > > >
