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

Fix codecov integration #191

Merged
merged 4 commits into from
Apr 25, 2024
Merged

Fix codecov integration #191

merged 4 commits into from
Apr 25, 2024

Conversation

devmotion
Copy link
Contributor

#188 broke codecov integration: Version 4 of the action does not support tokenless iploads anymore. Thus currently codecov uploads will always fail, see eg https://github.com/kalmarek/Arblib.jl/actions/runs/8724232667/job/23934347761#step:8:34. This error was masked by the fail_ci_if_error: false, so you might want to consider changing it to true (at the cost of spurious test failures sometimes since codecov servers are not completely reliable in my experience).

To fix codecov integration, in addition to this PR you have to go to the codecov webpage, navigate to the settings for this repo, and copy the token shown there to a Github repo secret named CODECOV_TOKEN.

@kalmarek
Copy link
Owner

@devmotion Thanks for detailed info and instructions!

I've added the token to the repo. I'd personally change fail_ci_if_error to true, but let @Joel-Dahne voice his opinion on this;)

@Joel-Dahne
Copy link
Collaborator

Changing fail_ci_if_error to true sounds reasonable to me as well!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@kalmarek
Copy link
Owner

kalmarek commented Apr 18, 2024

@devmotion and now macOS fails as it tries to use (the non-existent) token from your fork? the other ones just resolve to tokenless upload. Is this ok to be merged?

EDIT: seems that the coverage was updated on codecov successfully!

@devmotion
Copy link
Contributor Author

An alternative to true could be something like JuliaStats/StatsBase.jl#920 (which was motivated by test failures in PRs from forks that we saw in StatsBase).

@kalmarek
Copy link
Owner

I'm fine with simple true; the more complicated it is the more maintenance burden it becomes

@kalmarek kalmarek merged commit 375a537 into kalmarek:master Apr 25, 2024
12 of 13 checks passed
@devmotion devmotion deleted the patch-1 branch April 25, 2024 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants