-
-
Notifications
You must be signed in to change notification settings - Fork 402
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(.github/codecov): Install deps for fips codecov #2023
base: main
Are you sure you want to change the base?
Conversation
It seems from the CI failure that code coverage builds for Windows for More resources:
@mxinden @Ralith @djc I guess a decision has to be made if full coverage for |
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.
What goal are you trying to achieve here?
I think we should definitely fix it so coverage builds can succeed (I'm surprised the main CI failures don't generate any notifications for me...). I think rustls has this working so it shouldn't be too hard to fix? https://github.com/rustls/rustls/blob/main/.github/workflows/build.yml#L51 |
6d859e2
to
80a485b
Compare
@djc Should be good to go now. Checks are passing: https://github.com/quinn-rs/quinn/actions/runs/12588265064 I can drop the last commit (meant to be temporary just for testing) if this looks good. |
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.
Yeah just copying what Rustls is doing makes sense, and the second commit which makes the coverage CI run in PRs combined with the fact that CI passed for this PR seems to indicate that the change works. Thank you for working on this irksome thing!
(We must remember not to merge until the second commit is removed.)
An attempt to fix https://github.com/quinn-rs/quinn/actions/runs/11562272211/job/32183071070 on
main
and (unreleased)0.5.6