feat: no-std #42
feat: no-std #42
Conversation
.github/workflows/ci.yml
Outdated
| echo 'FEATURES: serde' | ||
| cargo clippy --no-default-features --features=serde | ||
| echo 'FEATURES: std, serde' | ||
| cargo clippy --no-default-features --features=std,serde |
There was a problem hiding this comment.
--all-features supports the same thing. Why do we need this?
There was a problem hiding this comment.
We are not testing --all-features, we are testing every combination of features. Explicitly listing them would be better.
Besides, --all-features would include libcpp, libcpp currently doesn't work. Fixing libcpp is not the purpose of this PR.
There was a problem hiding this comment.
Can you open an issue for libcpp?
test.sh
Outdated
| msg 'TEST: serde' | ||
| cmd cargo test --no-default-features --features=serde | ||
| cmd cargo clippy --no-default-features --features=serde | ||
| cmd env RUSTDOCFLAGS='-D warnings' cargo doc --no-default-features --features=serde |
There was a problem hiding this comment.
Is there a more elegant way of testing these?
There was a problem hiding this comment.
Is there a more elegant way of testing these?
I suppose you can write a script that reads Cargo.toml and create every feature combination and loop over them? But that would be overly complicated. Our time is better spent on the code of the library itself.
We can also reduce the number of combination by making serde requires std, or merging serde into std.
(BTW, --all-features does not replace this script, --all-features is but one amongst the possible combinations).
But if what you meant is you don't like .sh file then we can switch to justfile or Makefile (preferably on another PR).
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Can you move this file to .github folder and rename it to something like test-all-features.sh?
There was a problem hiding this comment.
Also cargo hack supports this https://github.com/taiki-e/cargo-hack#--feature-powerset
There was a problem hiding this comment.
Can you move this file to
.githubfolder and rename it to something liketest-all-features.sh?
This script is not actually used by the CI. It is used by me in the terminal.
Also
cargo hacksupports this https://github.com/taiki-e/cargo-hack#--feature-powerset
Does it support doc checking and clippy linting?
|
cc @steveklabnik can you review too? |
|
I would love to but cannot guarantee that I can until Sunday. Maybe earlier but no promises, sorry about that! |
There was a problem hiding this comment.
It looks good to me, although we should wait for @steveklabnik's review before merging this, and releasing a new version
|
Agree. Can you fix the conflict in this pr @KSXGitHub? I'll release the next version as v2 |
I just pushed the change as the same time as you made this request, it seems. |
closes #41