-
Notifications
You must be signed in to change notification settings - Fork 1
Push code coverage to codecov #21
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
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
dan-fernandes
left a comment
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.
Once bogus test is removed, LGTM 👍
tpoliaw
left a comment
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.
Can we add some config to avoid the annoying things like moving untested code causing a CI run to fail?
For numtracker I've gone for making the patch check informational only, and given a bit of leeway in the project coverage so dropping slightly is not a blocker. We should still aim for high coverage but adding pointless tests to raise coverage levels isn't the best use of time.
This repo doesn't have any rules defined for what checks must pass for a PR to be mergable. I propose adding one with format/lint/test only? |
I did not add any config to make the coverage information is because currently it is just information in both numtracker and glazed because they are not merge blockers(required) It just gives the reviewer some warning before merging to see if there were some test that could have been added which are missing ..rather than adding mockeypatch tests just to increase coverage. Happy to add the information codecov in CI to make it information but I think then it should added to the python copier template as well to make it a Diamond wide policy |
402c4fa to
0dc66af
Compare
39ecda8 to
a43f325
Compare
|
@abbiemery Are you okay with this going in ? I was told by @dan-fernandes to check with you |
c68964f to
6d3c314
Compare

No description provided.