refactor(rust-port): remove unused once-cell crate#58
Conversation
|
Hi can you review @ruvnet, this is my first time contributing to the project |
ruvnet
left a comment
There was a problem hiding this comment.
Review
The once_cell removal is correct. Confirmed once_cell, Lazy, and OnceCell are not referenced anywhere in crates/wifi-densepose-nn/src/. The Cargo.toml change is a clean 1-line removal. Good catch.
However, the Cargo.lock regeneration introduces concerns:
The lockfile was regenerated with cargo build --release, which bumped ~30+ transitive dependencies to their latest versions and added 15 new crates that weren't in the previous lockfile:
getrandom 0.4.1, id-arena, leb128fmt, prettyplease, unicode-xid,
wasip3, wasm-bindgen-test-shared, wasm-encoder, wasm-metadata,
wasmparser, wit-bindgen-core, wit-bindgen-rust, wit-bindgen-rust-macro,
wit-component, wit-parser
These are transitive deps pulled in by version bumps (likely wasm-bindgen 0.2.100 → 0.2.103 bringing in WASI P3 / WIT component model support). No crates were removed.
The version bumps are individually fine (anyhow, chrono, clap, futures, etc. are all patch/minor bumps), but bundling them with a "remove unused crate" PR makes bisection harder if a regression surfaces.
Suggestion: Consider splitting into two commits or at minimum noting in the PR description that the lockfile includes transitive version bumps beyond the once_cell removal. This helps reviewers understand the scope.
Summary:
- Cargo.toml change: approved — clean, correct
- Cargo.lock: functionally fine but scope is larger than described (30+ version bumps, 15 new transitive crates)
- No CI checks configured to validate the build — suggest running
cargo test --workspacelocally before merge
|
@ruvnet this is because the Cargo.lock is outdated, so regeneration is needed so i can fully remove |
Summary of changes
once-cellcrateCargo.lockby runningcargo build --release