+1 but just as an MXNet lover newbie :) We at Apache Struts, already use "coveralls" and I personally like it a lot even with some limitations with it. It doesn't allow us to merge a PR with a not tested code. It reports us where are those not covered new codes.
Regards. On 6/21/2018 7:13 PM, Marco de Abreu wrote: > To avoid any confusion, I have disable the PR comments at > https://github.com/apache/incubator-mxnet/pull/11344/files#diff-1f7b4b85ed8525c5239f741431a72872R25. > Thus, the data will only be recorded in the background is then retrievable > at https://codecov.io/gh/apache/incubator-mxnet/pulls. > > -Marco > > On Thu, Jun 21, 2018 at 6:43 AM Marco de Abreu <[email protected]> > wrote: > >> 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 <[email protected]> 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 <[email protected]> >>> wrote: >>> >>>> we should have a betting pool on what the current coverage is. >>>> >>>> On Wed, Jun 20, 2018 at 7:54 PM Naveen Swamy <[email protected]> >>> wrote: >>>> >>>>> +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 >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
