Developer experience - enable all the features when running tests by default #2516
Conversation
| @@ -77,6 +77,11 @@ parking_lot = "0.12" | |||
| rand = "0.8" | |||
| tempfile = "3.7.1" | |||
| semver = "1.0.7" | |||
| nix = { path = ".", features = ["acct", "aio", "dir", "env", "event", "fanotify", | |||
There was a problem hiding this comment.
IIUC, this dev dependency is for tests (examples/benchmarks), so I guess we cannot remove the --all-features flag of cargo build and cargo clippy?
There was a problem hiding this comment.
Both work for me locally, CI passes.
There was a problem hiding this comment.
Yeah, that's right, but that means it only builds or runs the clippy check for a small set of code, which is not gated with any feature
There was a problem hiding this comment.
It was still needed for build and clippy targets, added it back.
|
Good point. Will check what's going on with it tomorrow.
…On Mon, Sep 23, 2024, 21:29 SteveLauC ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Cargo.toml
<#2516 (comment)>:
> @@ -77,6 +77,11 @@ parking_lot = "0.12"
rand = "0.8"
tempfile = "3.7.1"
semver = "1.0.7"
+nix = { path = ".", features = ["acct", "aio", "dir", "env", "event", "fanotify",
Yeah, that's right, but that means it only builds or runs the clippy check
for a small set of code, which is not gated with any feature
—
Reply to this email directly, view it on GitHub
<#2516 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAQFIZKGZNDFYBHZ6ZP64LZYC57HAVCNFSM6AAAAABOXFIMHWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMRTHAYTMMZRGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
pacak
force-pushed
the
all-feats
branch
2 times, most recently
from
9931757 to
8b90ec9
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
The CI error that blocks this PR from being merged is fixed in #2518, once it is merged, you can rebase your branch:) |
|
#2518 is merged. |
SteveLauC
changed the title
| @@ -57,9 +57,9 @@ task: | |||
| << : *TEST | |||
| i386_test_script: | |||
| - . $HOME/.cargo/env | |||
| - cargo build --target i686-unknown-freebsd --all-features | |||
There was a problem hiding this comment.
Looks like this was reverted accidentally
There was a problem hiding this comment.
Hmmm... Good catch. I wonder how I managed that. Time to go to sleep I guess.
pacak
changed the title
What does this PR do
#2091 left developer experience in a bad state: as far as tools are concerned by default - the project is broken.
cargo testfails with lots of bogus errors,rust-analyzerignores majority of the code, etc.This PR tries to address this problem by explicitly enabling all the features for the
nixcrate in dev mode and making sure they will be added manually to the list by removing explicit--all-featuresfrom a few places.According to cargo docs path-only dev dependencies are allowed and will be ignored when publishing the crate.
With this change in place I can run tests with usual
cargo test,rust-analyzerand other tools work as expected.Checklist:
CONTRIBUTING.md