-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
chore: switch to c8 for coverage #8
Conversation
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 cool but I think there are some easy improvements to make. If c8 report
isn't a thing, maybe using the lcov
reporter in the test command would be more efficient
.travis.yml
Outdated
language: node_js | ||
node_js: | ||
- "10.10" | ||
after_script: npm run 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.
I would recommend using after_success
here.
package.json
Outdated
"posttest": "standard", | ||
"release": "standard-version" | ||
"release": "standard-version", | ||
"coverage": "c8 --reporter=text-lcov tap test/*.js | coveralls" |
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.
Sorry if I'm missing context here, but is there a c8 report
instead of having to run all the tests again?
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.
I haven't hammered out c8 report yet, but I'd be happy to do so before landing this.
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.
switches to using c8 for coverage now that coverage support has been landed in Node.js
10.10.0
.