-
-
Notifications
You must be signed in to change notification settings - Fork 634
🧪 Improve Codecov integration stability #2265
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
🧪 Improve Codecov integration stability #2265
Conversation
6ab7e4d to
97aa041
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.
👍 for the core ideas. I've not used this codecov mechanism before, but I'm happy to learn.
|
|
||
| comment: false # avoid spamming reviews | ||
|
|
||
| ... |
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.
Is an ellipsis a special kind of YAML node? I don't recall this one, but there are so many exotic features in YAML that I'm slow to assume that it's a mistake.
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.
It's just a marker of the end. Totally optional. I just like it there and it got copied from another project.
A multi-document YAML file might look like this:
---
doc1: stuff
---
doc2: things
---
doc3: the rest
...
I've only started integrating it recently. I've added it to ansible/ansible and aio-libs/yarl only, I think. Previously, I was adding the exact number of uploads to the config to prevent Codecov from reporting too early. But that's annoying to maintain as it should match the number of CI jobs which changes with the matrix. So explicit notifications is sorta the same but not precisely. AFAIU, this delays Codecov's processing in their backend until notified. However, I think it still may sometimes report checks (statuses/comments) to GH earlier than it finished processing all the reports it'd received. The linked PRs have a bunch of references that I explored — they made me conclude it works like this. In other news, I started this because the codecov upload step gets skipped on failures and I'm currently trying to fix that — GHA seems to evaluate the conditional to |
8656554 to
53b2eb5
Compare
This is achieved by checking a `cancelled()` state which makes sure the default `success()` isn't applied.
This aims to prevent Codecov from processing coverage reports too early.
a4d4450 to
17050d9
Compare
17050d9 to
3e234d9
Compare
3e234d9 to
b0730a6
Compare
The patch aims to ensure Codecov reports are uploaded on failures. This is achieved by checking a
cancelled()state which makes sure the defaultsuccess()isn't applied.It also bumps the Codecov action to v5 and makes the Codecov config dot-prefixed.
Finally, an explicit notification mechanism is configured to tell Codecov about upload completions. This should to prevent Codecov from processing coverage reports too early.
Contributor checklist
changelog.d/(seechangelog.d/README.mdfor instructions) or the PR text says "no changelog needed".Maintainer checklist
bot:chronographer:skiplabel.