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

Reply via email to