Skip to content
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

Switches to codecov GH action #1118

Merged

Conversation

m1kola
Copy link
Member

@m1kola m1kola commented Jun 26, 2023

Description of the change:

Switches the repo to use codecov GH action.

Motivation for the change:

In #639 (after Codecov bash uploader vulnerability) this repo was switched to use copy-pasted bash script.

Since then many things changed:

I think it is safe to switch back to this GH action.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 26, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@@ -13,10 +13,13 @@ jobs:
unit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
Copy link
Member Author

@m1kola m1kola Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to update it with rest of the actions in #1117, but .github/workflows/codecov.sh fails with v3. As far a s I understand, bash script relies on older version of node which was provided by actions/checkout@v2 and no longer works with Node 16.

@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Merging #1118 (5c43e7b) into master (6c60284) will increase coverage by 1.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1118      +/-   ##
==========================================
+ Coverage   52.64%   53.80%   +1.15%     
==========================================
  Files         108      108              
  Lines        9703    10163     +460     
==========================================
+ Hits         5108     5468     +360     
- Misses       3653     3734      +81     
- Partials      942      961      +19     

see 63 files with indirect coverage changes

@m1kola m1kola force-pushed the codecov_gh_action branch 2 times, most recently from d01e47a to 1e11eb2 Compare June 26, 2023 15:35
- uses: codecov/codecov-action@v3
with:
files: coverage.out
fail_ci_if_error: true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Equivalent of -Z from the bash script.

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
with:
files: coverage.out
fail_ci_if_error: true
functionalities: fixes
Copy link
Member Author

@m1kola m1kola Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to avoid things like on the screenshot which decrease code coverage (bash uploader was doing it by default).

I believe this doc is related: https://docs.codecov.com/docs/fixing-reports#go

Screenshot 2023-06-26 at 16 47 36

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can remove this line and estabilish a new baseline in master without enabling these fixes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, good with either approach. Using 'fixes' right now seems generally good?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think fixes gives exactly the same result as it was with the previous uploader. After adding fixes the diff between master coverage and this PR is empty (see results here).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After adding fixes the diff between master coverage and this PR is empty (see results here).

Ah, interesting. Yesterday after running the tests the diff was empty and now it says +1.16%. Probably codecov was still processing when I opened the link.

Anyway - I think we should keep fixes for now.

@m1kola m1kola marked this pull request as ready for review June 26, 2023 16:16
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2023
@m1kola m1kola requested a review from grokspawn June 26, 2023 16:17
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grokspawn, m1kola

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2023
@grokspawn
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2023
@openshift-merge-robot openshift-merge-robot merged commit b854557 into operator-framework:master Jun 27, 2023
12 checks passed
@m1kola m1kola deleted the codecov_gh_action branch June 27, 2023 18:33
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants