-
Notifications
You must be signed in to change notification settings - Fork 791
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
Switch from Coveralls to Codecov #646
Conversation
514760b
to
1d37cc5
Compare
Codecov Report
@@ Coverage Diff @@
## master #646 +/- ##
========================================
Coverage ? 91.4%
========================================
Files ? 31
Lines ? 1978
Branches ? 326
========================================
Hits ? 1808
Misses ? 90
Partials ? 80
Continue to review full report at Codecov.
|
Looks like it's now working well pushing from circleci to codecov without needing a token to be set, so it will work on PRs from forks :) https://codecov.io/gh/ethereumjs/ethereumjs-vm/pulls @holgerd77 if this looks good to merge, please update the check from ghactions-coverage to circleci-coverage and should be good to go! |
Just comparing the coverage results from Coveralls (right) and Codecov (left). Coverage is systematically lower on Codecov, I would guess that their metrics work a bit differently and there is a somewhat more strict count in place. Wouldn't make this a blocker, especially in this direction (lower count) this is relatively unproblematic since it doesn't give a false sense of security / good coverage. |
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.
This is really great, thanks a lot @ryanio! Really looking forward to the new coverage reports, confident that this will have a significant effect on test quality / coverage considerations along with PRs. This went always a bit under radar (unless someone wanted explicitly improve the coverage), since the info was very much inconvenient to consume along the normal work flow. So: cool! 😄
Regarding the lower percentage, although I don’t see it as a problem as well, we can test some other settings to see how it behaves. |
(Successor to #645)
Due to codecov/codecov-action#29, it does not look like GH Actions can post to CodeCov on PRs from forks due to the inability to share secrets.
CircleCI does not need
CODECOV_TOKEN
, so this PR tests pushing the coverage reports using CircleCI.Once codecov/codecov-action#29 is resolved, we can remove the use of CircleCI and re-enable
.github/workflows/vm-coverage.yml
with the proper fix.