Lint the experimental feature in CI - #720
Merged
Merged
Conversation
make pre only ran clippy with the default and empty feature sets, neither of which enables experimental, so AsyncFilesystem was never linted. Its read method takes the same nine arguments as Filesystem::read, which too_many_arguments rejects, and CI sets RUSTFLAGS=--deny warnings, so anyone running clippy over that feature hit a hard error the build never reported. Allow the lint on the trait, as Filesystem already does, since both mirror the argument list of the FUSE read request. Add a clippy run with --all-features to pre, which also covers serializable for the first time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NjYWzn3mu8Sc2y2eACtJM9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
make preran clippy with the default and empty feature sets, and neither enablesexperimental, soAsyncFilesystemhad never been linted.serializablewas uncovered for the same reason.Running clippy over that feature hits a hard error, since CI sets
RUSTFLAGS=--deny warnings:AsyncFilesystem::readtakes the same nine arguments asFilesystem::read, because both mirror the argument list of the FUSE read request.The fix
#[allow(clippy::too_many_arguments)]on the trait, matching whatFilesystemalready carries atsrc/lib.rs:467. The sync twin of this trait made exactly this call for exactly this signature; applying it inconsistently is what left the async trait unlintable.Worth flagging that
AGENTS.mdsays to fix warnings rather than silence them, and this is the silencing branch of that rule. The alternative is folding the arguments into a params struct, which would be an API redesign that diverges the async trait from the sync one — out of proportion to a lint gap, and a decision better made on its own merits than as a side effect of turning the lint on.The CI change
Added
cargo clippy --all-targets --all-featuresto the Makefilepretarget, which is what thecijob runs. Keepingmake preand CI identical is the property whose absence made this invisible, so the new check belongs there rather than as a separate workflow step.On Linux
--all-featuresresolves to libfuse3 + experimental + serializable:build.rspicks libfuse3 before libfuse2, andmacos-no-mountonly applies to macOS targets, so the combination is coherent rather than a pile of mutually exclusive flags.Verification
The new gate is not vacuous — with the
allowstashed, it fails as intended:Also green under
RUSTFLAGS=--deny warnings: all three clippy variants,cargo fmt --check,cargo test --all --all-features(85 + 2 pass), andcargo doc --all --no-deps --all-featureswithRUSTDOCFLAGS=--deny warnings.The new invocation adds ~3s incrementally. The
cijob runs in roughly 35s warm against a 10-minute timeout, so there is headroom even on a cold cache that has to build tokio and serde for the first time.No CHANGELOG entry: this changes no behavior and no API.
cargo deny check licenses, the one part ofprenot run locally, is unaffected — no dependencies changed.🤖 Generated with Claude Code
https://claude.ai/code/session_01NjYWzn3mu8Sc2y2eACtJM9
Generated by Claude Code