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

Fix --no-default-features test warnings; consolidate CI jobs #461

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

tarcieri
Copy link
Contributor

@tarcieri tarcieri commented Dec 8, 2022

Previously cargo test --no-default-features would succeed but with warnings.

This commit fixes all of those warnings and tests --no-default-features in CI to ensure that in perpetuity.

It moves the tests-default-serde into the test job which reduces the total number of job runners needed and reuses intermediate artifacts which should improve build times.

It removes test-alloc-u32 as this case is already covered by the test job when it runs cargo test --target i686-unknown-linux-gnu as the alloc feature alone is now enabled by default.

@tarcieri tarcieri requested a review from rozbb December 8, 2022 20:34
@tarcieri tarcieri mentioned this pull request Dec 8, 2022
@tarcieri tarcieri force-pushed the fix-warnings-and-consolidate-ci-jobs branch from 06be731 to b4b3083 Compare December 8, 2022 20:37
@rozbb
Copy link
Contributor

rozbb commented Dec 8, 2022

This commit fixes all of those warnings and tests --no-default-features in CI to ensure that in perpetuity.

Is there a flag somewhere that errors if it sees a warning?

@tarcieri
Copy link
Contributor Author

tarcieri commented Dec 8, 2022

It’s -D warnings which I now see we don’t have enabled in CI. Let me go ahead and add that.

Previously `cargo test --no-default-features` would succeed but with
warnings.

This commit fixes all of those warnings and tests
`--no-default-features` in CI to ensure that in perpetuity.

It moves the `tests-default-serde` into the `test` job which reduces the
total number of job runners needed and reuses intermediate artifacts
which should improve build times.

It removes `test-alloc-u32` as this case is already covered by the
`test` job when it runs `cargo test --target i686-unknown-linux-gnu` as
the `alloc` feature alone is now enabled by default.
@tarcieri tarcieri force-pushed the fix-warnings-and-consolidate-ci-jobs branch from b4b3083 to fe6a4b9 Compare December 8, 2022 22:30
@@ -8,6 +8,7 @@ on:

env:
CARGO_TERM_COLOR: always
RUSTFLAGS: '-D warnings'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added -D warnings.

FWIW ed25519-dalek already has it

@tarcieri
Copy link
Contributor Author

tarcieri commented Dec 8, 2022

@rozbb added -D warnings if you want to re-approve / merge

@rozbb rozbb merged commit 1e490bd into release/4.0 Dec 9, 2022
@tarcieri tarcieri deleted the fix-warnings-and-consolidate-ci-jobs branch May 31, 2023 02:13
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.

2 participants