-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Set fail_ci_if_error flag to true #6372
Set fail_ci_if_error flag to true #6372
Conversation
I expect the first build here to be red, and after that I will follow the instructions at https://docs.codecov.com/docs/adding-the-codecov-token and then it should be green 🤞 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Finally! A build that fails because of Codecov |
Ah but secrets are just not passed to workflows triggered from a fork, so there is no way this is going to work, is there? 😞 |
Looks like v4 addresses this ? |
eb26cac
to
02f0276
Compare
Wonderful increase 😁 Looking at the CodeCov UI, it says there is only 1 upload, but when I download it, I can see it contains several reports. Note that we might have to specify |
In a recent PR, we got reports from CodeCov about several lines not being covered. After further inspection, I found an error message in the coverage file upload saying we hit a rate limit. This will make it more likely we spot such issues in the future. The PR: doctrine#6370
02f0276
to
27eb261
Compare
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'd expect that error to appear again. Should codecov upload on a PR? What about having just a coverage % output on a PR and the upload on merges to the branches? Food for thoughts if another PR is needed.
IMO it can be useful for us to see what part of a patch is uncovered before merging, so yes. If the error appears again, that won't prevent a merge, and it's IMO better for us to be aware of the issue rather than getting lots of comments about lines being uncovered. |
In a recent PR, we got reports from CodeCov about several lines not being covered. After further inspection, I found an error message in the coverage file upload saying we hit a rate limit.
This will make it more likely we spot such issues in the future.