singa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [singa] moazreyad commented on issue #796: codecov/project fail
Date Tue, 15 Sep 2020 11:38:55 GMT

moazreyad commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692659125


   There is a problem in this PR because it [installs singa in python 3.6 ](https://github.com/nudles/singa/blob/dev/.github/workflows/conda.yaml#L64)
and [executes the test cases from python3.7/site-packages](https://github.com/nudles/singa/blob/dev/.github/workflows/conda.yaml#L66)
folder which does not work. Please fix this different python versions problem.
   
   Note also that the project coverage threshold can be set in [.codecov.yml](https://github.com/apache/singa/blob/dev/.codecov.yml)
like [this](https://docs.codecov.io/docs/commit-status#project-status):
   
   ```
   coverage:
     status:
       project:
         default:
           # basic
           target: auto
           threshold: 5%
   ```
   The value of threshold can be chosen by the development team based on the quality needs.
In the best case, every PR should not reduce the coverage because this means it either added
code without test case or it removed (or disabled) the test cases for existing code. However,
if the team think this is too strict requirement, we may choose the threshold to 5% or even
more. But this means that a PR can be allowed to pass even if it reduces the code coverage
too much. 
   
   The PR #795 reduces the coverage by 4.97%, so setting the threshold to 5% should make it
pass. But this is not recommended solution. We can use it only if the team thinks it is too
strict to keep the coverage change always zero or positive. In this case, we may allow threshold
of 1% or 2% to pass. But 5% seems high to me and it means there is a lot of code that was
not tested and there is a problem. And thanks to the strict code coverage, we found the different
python versions problem. Without the strict coverage check, may be this problem will be hidden
and will cause other problems later.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message