-
Notifications
You must be signed in to change notification settings - Fork 42
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
Restore coverage publishing #516
Conversation
- now we send report using coveralls app
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.
Great job! I see the results at https://coveralls.io/jobs/136529785
Could you please check if we need to address my comments?
.github/workflows/ci.yml
Outdated
run: yarn run merge-cover | ||
|
||
- name: Coveralls | ||
if: matrix.node == '20' |
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.
The CI script runs twice, on push and PR. It uploads the coverage twice. Could we do it only once, on push?
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.
Good catch 😉
I limited publishing only for 'push' event.
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.
Thanks
Description
coveralls
package because publication is done via application (note :coveralls
removing fixesjson-schema
vulnerability https://github.com/epam/miew/security/dependabot/114)lcov-result-merger
,mkdirp
,rimraf
Type of changes
Checklist
yarn run ci
: lint and tests pass locally with my changes.