mxnet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marco de Abreu <marco.g.ab...@googlemail.com.INVALID>
Subject Re: [LAZY VOTE] Test coverage of PRs
Date Thu, 21 Jun 2018 13:43:06 GMT
Hi Kellen,

Thanks for your feedback.

The same argument about a hosted service could be applied to GitHub - we
could just use the git repository hosted under Apache Infra then and
completely stick to JIRA. This service gives us the advantage of being free
of charge, a mature project and having built-in support for all languages
we run in this project. If you got an open source software which offers the
same features and is as easy to manage, I'm happy to take suggestions.

As stated previously, we're going to start with Python and C++ coverage,
evaluate it over the following days/weeks and then discuss how we should
move forward. I'm happy to take contributions from the community to extend
the coverage across more languages. For now, I'd just like to gather some
data.

Best regards,
Marco


kellen sunderland <kellen.sunderland@gmail.com> schrieb am Do., 21. Juni
2018, 11:15:

> -0.1 (non-binding).
>
> Looks good, and I like the idea of adding coverage, however I have a few
> minor issues with this approach:
>
> First it seems to use a hosted service, which can disappear / be acquired /
> change terms at any time.  In general I would try and avoid using services
> like this for Apache projects.  I don't feel as strongly about this as some
> in the community do, but there are so many open source alternatives to
> generate coverage reports that I'm not sure why we have to go the Saas
> route in this case.
>
> Secondly the core library is written in C++.  Most of the significant bugs
> I've seen in MXNet so far have been in the C++ code.  I'd suggest we focus
> on coverage reports for C++ before, or at least in addition to Python.  I
> feel less strongly about the other languages, but coverage in other areas
> (i.e. Scala) might also be nice eventually.
>
> -Kellen
>
> On Thu, Jun 21, 2018 at 4:59 AM Chris Olivier <cjolivier01@gmail.com>
> wrote:
>
> > 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