-
Notifications
You must be signed in to change notification settings - Fork 36
Try to switch to CodeCov #381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #381 +/- ##
=========================================
Coverage ? 89.23%
=========================================
Files ? 52
Lines ? 1198
Branches ? 0
=========================================
Hits ? 1069
Misses ? 129
Partials ? 0 Continue to review full report at Codecov.
|
I think it's fine, mainly also because it simplifies the CI setup (I don't think the app installation is problematic). It seems the reported coverage is lower than the one reported by coveralls? |
Yep I think it's because of the parallelization of our tests |
Looking into this, it looks like CodeCov and Coveralls count the lines differently, it's a bit odd... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
"Others" failing because of JuliaStats/Distances.jl#221 (try/catch not compatible with Zygote) @devmotion what should I do here? Should I comment out the tests with SqMahalonobis? |
Maybe just upper bound Distances in test/Project.toml and open an issue so we don't forget to remove the bound once the problem is fixed upstream? |
Co-authored-by: David Widmann <[email protected]>
Summary
The integration of Coveralls is a bit annoying (need to add a Bot as a collaborator). Also it would be better to use the same coverage processing with all repos.
Proposed changes
Move from Coveralls to CodeCov
What alternatives have you considered?
Adding Coveralls bot as a member
Breaking changes
None