-
Notifications
You must be signed in to change notification settings - Fork 294
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
Added Codecov to CI Pipeline #820
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 great! I have a few questions
with: | ||
token: ${{ secrets.CODECOV_TOKEN }} | ||
files: ./code-coverage-report/build/reports/jacoco/codeCoverageReport/codeCoverageReport.xml | ||
if: steps.jacoco_report.conclusion == 'success' |
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.
Does steps.jacoco_report.conclusion != 'success'
if that step does not run? Just checking, as that step only runs on a certain OS, Java version, etc. (see line 69)
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.
Yes, this gets skipped if the previous step doesn't run. Initially I had the same conditions as the previous step, but that felt redundant for this step.
However, I just realized that the previous step has continue on error
as true, so I have changed this to use outcome
instead of conclusion
so that it runs iff the code coverage report succeeds. For conclusion == success
, it will run even if the step fails with an error which can cause issues uploading and break the workflow.
Removed token dependency, build cache clean and replaced outcome with conclusion
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #820 +/- ##
=======================================
Coverage ? 0
=======================================
Files ? 0
Lines ? 0
Branches ? 0
=======================================
Hits ? 0
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. |
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.
One final very minor comment but otherwise this looks great!
Co-authored-by: Manu Sridharan <[email protected]>
@armughan11 I think the next step is to land this PR, and then test things out by creating another PR (maybe one that reduces test coverage) from a fork. Do you agree? And if so, could you create that test PR after this one lands? |
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.
👍
@msridhar That sounds good! I can create that PR once this lands! |
Replaced Coveralls with Codecov for the CI pipeline
Fixes #499