-
Notifications
You must be signed in to change notification settings - Fork 119
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
Reduce warnings in GitHub workflows #9520
Conversation
cc7c4ff
to
c4fdc3f
Compare
@@ -167,7 +167,7 @@ jobs: | |||
-o ${{ runner.temp }}/artifacts/functional_lcov.info | |||
|
|||
- name: Upload coverage to Codecov | |||
uses: codecov/codecov-action@v3 | |||
uses: codecov/codecov-action@v4 |
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.
Apparently Codecov requires a token: codecov/codecov-action#1274 (comment)
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.
Someone needs to set up a secret (I have access to the key in Codecov, but not this repo).
https://docs.codecov.com/docs/adding-the-codecov-token#github-actions
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.
Honestly, we should just strip out the codecov checks, they haven't worked since Raphael left the team.
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.
Let me know if I should remove these steps, or merge this as is ;-)
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.
Merge it as-is. I can follow up with another PR to strip out the codecov stuff.
b74147e
to
bfc557a
Compare
This error is not due to updates, it's been there all the time (macOS tests). It's possible to set the action to not produce warnings, but someone needs to confirm if that's wanted behavior or not (no idea what should be in these folders). |
@@ -130,7 +132,7 @@ jobs: | |||
MVPN_BIN: ./build/dummyvpn | |||
|
|||
- name: Uploading artifacts |
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 couldn't help noticing that this looks very similar to the Linux workflow, which doesn't fail.
That workflow has an additional step though, using the same artifacts folder, so that might be the reason?
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4
if: steps.generateGrcov.outcome == 'success'
with:
directory: .
flags: functional_tests
name: codecov-poc
files: ${{ runner.temp }}/artifacts/functional_lcov.info
verbose: true
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 all looks like positive changes to me!
There are currently over 100 warnings in checks, mostly due to obsolete GitHub actions.