mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Naveen Swamy <mnnav...@gmail.com>
Subject Re: [LAZY VOTE] Test coverage of PRs
Date Thu, 21 Jun 2018 02:53:58 GMT
+1 to collect data on coverage.

On Wed, Jun 20, 2018 at 7:38 PM, Marco de Abreu <
marco.g.abreu@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 <tqchen@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.abreu@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
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message