mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Olivier <cjolivie...@gmail.com>
Subject Re: [LAZY VOTE] Test coverage of PRs
Date Thu, 21 Jun 2018 02:58:52 GMT
we should have a betting pool on what the current coverage is.

On Wed, Jun 20, 2018 at 7:54 PM Naveen Swamy <mnnaveen@gmail.com> wrote:

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