Codecov improvements #3156
Codecov improvements #3156
Conversation
|
@jakkdl flag errors is just an annoying UI thing. I reported it upstream some time ago. |
.github/workflows/ci.yml
Outdated
| @@ -87,7 +87,6 @@ jobs: | |||
| directory: empty | |||
| token: 87cefb17-c44b-4f2f-8b30-1fff5769ce46 | |||
There was a problem hiding this comment.
It's better to move this token to the config as it would be easier to keep it in one shared place.
There was a problem hiding this comment.
done!
looks like v4 & v5 has new tools for managing tokens & tokenless uploads, so if we wanna do it properly we could look at that later. But the minimal downside of exposing the token might not be worth the effort of doing that https://docs.codecov.com/docs/codecov-tokens#uploading-without-a-token
There was a problem hiding this comment.
v4 introduced OIDC, which would only work in trusted contexts (which PRs from forks aren't). They have corner case handling for such PRs.
With v5, it's possible to toggle tokenless uploads in the Codecov UI. Not sure if this is an org-only setting or it could be per-repo. I haven't tested it.
| @@ -87,7 +87,6 @@ jobs: | |||
| directory: empty | |||
| token: 87cefb17-c44b-4f2f-8b30-1fff5769ce46 | |||
| name: Windows (${{ matrix.python }}, ${{ matrix.arch }}${{ matrix.extra_name }}) | |||
| flags: Windows,${{ matrix.python }} | |||
There was a problem hiding this comment.
I'd keep the flags despite the UI alerts.
There was a problem hiding this comment.
I suppose we could keep them, but nobody has pressed the button to enable analytics so currently it's entirely unused anyway https://app.codecov.io/gh/python-trio/trio/flags
I still find it somewhat questionable to track coverage for "python3.11" or "windows" as separate entities. Maybe you could diagnose issues by seeing big swings in coverage for one of them*, or any of them being very low, but the actual coverage % for each flag is mostly useless unless we'd do something like marking all platform/version-specific code somehow, exclude from coverage for that flag, and require 100% coverage on common code on all platforms... (which all sounds like way too much effort for close-to-no payoff).
* could use https://docs.codecov.com/docs/commit-status#threshold
There was a problem hiding this comment.
I think I've used the flags once (when looking at updating the codecov action) but I don't remember how + given we haven't figured the issue out I assume it wasn't useful.
There was a problem hiding this comment.
I guess 1>0 and there's not really any downside to have them, so I'll add them back in. I'll go enable analytics too for fun.
There was a problem hiding this comment.
I use them heavily in many projects. It's nice to see which likes are covered under which env or a combo of those. There's a way to filter it on the Codecov UI.
As for the checks and 100% coverage requirement, I usually don't make those depend on the flags, except for MyPy, perhaps. Codecov is able to track the combined coverage for those checks. I can look into improving this later.
There was a problem hiding this comment.
It's a bit of a pain that https://app.codecov.io/gh/python-trio/trio/flags shows all flags ever used, with 1/3 being for old versions we no longer support. I'm not sure there's a decent way to fix that without doing manual flag management
There was a problem hiding this comment.
Yeah, you can clean them up. Although, to me, the most useful view with flags is the per-file line coverage pages.
we had a lot of troubles with v4 #2951 |
v4 had a lot of bugs when it was first released, half of the reports on the first day of the release were probably mine. It should be stable now. It was another uploader rewrite in Python (v3 was JS), and v5 has yet another wrapper so it feels a bit more risky to rush into it. |
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
There was a problem hiding this comment.
I haven't noticed anything blocking so let's merge this. Not sure why macos is failing, though. It should be unrelated, right?
I'd like to do some follow-up improvements post-merge since I've been doing this across a number of projects anyway. Might as well transfer the lessons learned here later.
|
Yea macos run is blocking #3155 as well, and A5rocks found some details about potentially why: #3155 (comment) |
Fixes #2689 (see #3153), although I'm not 100% sure this is still a problem - or if I've just gotten used to marking failed PR runs as read in my notifications. I'll go test it EDIT: This is still a thing - if you're watching runs in progress I'm getting a ❌ (and it was never about notifications anyway)
#2653 added flags, but configured them erronously with multiple flags per run, so we've been getting lots of flag errors - see e.g. https://app.codecov.io/gh/python-trio/trio/commit/785dc59309ca4f491562b817836092f54b7da4b1
But I'm not sure if flags are terribly useful for us, e.g. wanting 100% coverage on windows is not something we care about, so I removed them.
I enabled codecov.notify.require_changes so it won't add a comment if there's no changes.
And finally I set the target for patches to 100%, where it by default targets project cov (currently 99.68%) which meant that in big PR's you could by mistake add an uncovered line.
Most settings inspired by https://github.com/aio-libs/yarl/blob/f304dd7/.codecov.yml, thanks @webknjaz