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

ci(rust.yml): add job testing feature permutations #1934

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

mxinden
Copy link
Contributor

@mxinden mxinden commented Jul 23, 2024

This pull request adds a GitHub job, testing all feature permutations (including optional dependencies), across all default workspace members. It uses cargo hack.

This workflow would for example have caught the following recent bugs before landing in main:

Right now this job takes 5m 1s on Ubuntu, 4m 42s on MacOS and 8m 29s Windows. In case you consider that too long, we could drop --optional-deps or exclude some features (--exclude-features).

@mxinden mxinden force-pushed the github-features branch 9 times, most recently from 3e50571 to 1b121e6 Compare July 23, 2024 15:21
@mxinden mxinden force-pushed the github-features branch 3 times, most recently from 96523a6 to 34388f8 Compare August 5, 2024 09:27
@mxinden mxinden changed the title ci(rust.yml): add workflow testing feature permutations ci(rust.yml): add job testing feature permutations Aug 5, 2024
@mxinden mxinden marked this pull request as ready for review August 5, 2024 09:50
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay, and the runtime is probably worth it.

Thanks for working on this!

.github/workflows/rust.yml Outdated Show resolved Hide resolved
@mxinden
Copy link
Contributor Author

mxinden commented Aug 18, 2024

@djc I did the rename. Mind taking another look?

@mxinden mxinden requested a review from djc August 18, 2024 14:26
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Ralith
Copy link
Collaborator

Ralith commented Aug 20, 2024

Looks like this needs a rebase.

@djc djc enabled auto-merge August 20, 2024 16:20
auto-merge was automatically disabled August 20, 2024 18:42

Head branch was pushed to by a user without write access

@Ralith Ralith added this pull request to the merge queue Aug 20, 2024
Merged via the queue into quinn-rs:main with commit edf16a6 Aug 20, 2024
13 checks passed
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