-
Notifications
You must be signed in to change notification settings - Fork 208
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
Migrating from vx -> v5 #1645
Comments
Where can I find it for a "single repository"? I can find only a global "Token authentication" option for the whole organization under Related PR: signalfx/splunk-otel-go#3496 I also noticed that the |
From https://docs.codecov.com/docs/codecov-tokens#uploading-without-a-token:
I find this description not clear.
Besides, both of these scenarios are not well suited for public open-source repositories. (2) does not seem to be true. Reference PR: open-telemetry/opentelemetry-go#5979 I noticed that for forks it does not seem to be a problem for |
This was my mistake. It is in fact for the whole organization. However, private repositories will always need a token to authenticate. I have updated this issue and the README. I apologize for the misinformation.
That is strange, can you link me to corresponding CI runs here? It shouldn't be different.
To clear things up, we do not mean
|
Putting GH run logs from open-telemetry/opentelemetry-go#5964 PR (https://github.com/open-telemetry/opentelemetry-go/actions/runs/11842872629/job/33002565858) at the end of this comment. At that time the org had token authentication option set to "Required" . Possibly fixed by #1650?
|
Does it mean that even with authentication option set to "Not required" we still need a token here: open-telemetry/opentelemetry-go#5979? If so then, I am not following what has changed regarding token authentication 🤷 |
This change makes sure that an unsuccessful upload will fail the workflow as this does not happen at the moment. See: https://github.com/open-telemetry/opentelemetry-go/actions/runs/11845773487/job/33011997881?pr=5978#step:3:175 Additionally, it configures the codecov action to be tokenless. Related issue (most likely solved): - open-telemetry/community#2440 From https://github.com/codecov/codecov-action?tab=readme-ov-file#migration-guide: > The v5 release also coincides with the opt-out feature for tokens for public repositories. In the repository settings page in codecov.io, you can set the ability for Codecov to receive a coverage report from ANY souce. This will allow contributors or other members of a repository to upload without needing access to the Codecov token. More codecov/codecov-action#1645 v4 was still working. See: https://github.com/open-telemetry/opentelemetry-go/actions/runs/11842872629/job/33002565858#step:3:11 We might also consider reverting to v4 of the codecov action and just setting `fail_ci_if_error: true`.
My dependabot just updated codecov from 4 to 5: maplibre/maplibre-gl-js#5050 In both the dependabot PR and the newly created one I don't see codecov coverage message. |
@HarelM, see https://github.com/maplibre/maplibre-gl-js/actions/runs/11853207459/job/33032875399#step:8:168
You are not using the |
@thomasrockhu-codecov, if so then the documentation should improved as currently it does not have any sense If I guess correctly this option only applies for public repositories to set whether codecov accepts tokenless reports or not (when the report is send from the actual repository and not from a fork). But maybe my guess is bad. |
Is the previous token configuration not valid anymore? There is a codecov token configured for this repo and PR... |
Bottom line, I think the migration/upgrade instructions can be improved given the issues that were recently opened and the discussion here. I would also argue that a CI that can fail should not have a default of continue with any notification and that it should fail the CI pipeline unless told otherwise, but that might just be me... |
I have another general question about the v5. I've read the diff, and I'm concerned about the change from a node-based action, that is supposed to be cross-platform (until it doesn't), whilst the new action is composite-based, and relies heavily on bash, and other bash features. How well does it work on other platforms? I still expect some success on Windows GitHub-hosted runners, as they include a git bash, but what about self-hosted runners? For macOS, on a project I'm working on we couldn't use code coverage yet, as the uploading action relied on some gpg binaries being available with homebrew, but for our software to build, we need to remove some installed software to use another toolchain, and even though the same software is installed, the path was hardcoded to the original path. Will v5 behave differently? |
@pellared yes, I'm going to update the documentation today as it seems that it's quite unclear. Would you be able to see if
That is correct, I need to make an update to the product team for this change, but it is only relevant to public repositories. |
@HarelM I saw this PR you made. I just pushed
💯, although it should have been more clear before we launched, I will spend my day improving the pieces that I can.
This is definitely something we have discussed a lot within the team. The gist of it is that we have users that have extremely long-running CI builds that don't necessarily rely on Codecov to merge. If there is a failure at that stage, it can be extremely painful for users. As a result, Codecov has always made this a non-blocking step by default in case of our own downtime or irregularities. We have been building and have built a lot of safeguards to prevent such issues and are leaning towards flipping it in a future major release of this Action |
@echoix, our expectation is that the As for |
Looks to be fixed: https://github.com/signalfx/splunk-otel-go/actions/runs/11857558947/job/33046215151#step:5:159. Thanks. |
@thomasrockhu-codecov I didn't see the status checks in the renovate PR to update this, as we don't enable PR comments, and only look at the status checks there. That made me uncertain if the upload and tokens worked, I was suspicious of the action update. I have my fork that also has renovate enabled, and also codecov configured with a separate token, and was able to merge it to my main, and in there, once merged, the status checked were there. |
@echoix got it, I think I found the root cause, working on a fix now |
@echoix ok, I pushed |
Let's see that in action once OSGeo/grass#4704 updates itself |
Seems like it returned to work, thanks! |
v5 Release
v5
of the Codecov GitHub Action will use the Codecov Wrapper to encapsulate the CLI. This will help ensure that the Action gets updates quicker.Migration Guide
The
v5
release also coincides with the opt-out feature for tokens for public repositories. In theGlobal Upload Token
section of the settings page of an organization in codecov.io, you can set the ability for Codecov to receive a coverage reports from any source. This will allow contributors or other members of a repository to upload without needing access to the Codecov token. For more details see how to upload without a token.Warning
The following arguments have been changed
file
(this has been deprecated in favor offiles
)plugin
(this has been deprecated in favor ofplugins
)The following arguments have been added:
binary
gcov_args
gcov_executable
gcov_ignore
gcov_include
report_type
skip_validation
swift_project
You can see their usage in the
action.yml
file.If you are having troubles with the migration, please open a new issue and tag @codecov/report-upload
The text was updated successfully, but these errors were encountered: