-
Notifications
You must be signed in to change notification settings - Fork 60
feat(scripts): consolidate external deps in cargo_sort.py
#9637
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
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 6 Skipped Deployments
|
semenov-vladyslav
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default-features = false in a member crate dependency has no effect and may be misleading. Default features are enabled/disabled in workspace manifest.
For example, if two member crates have a common dependency, but one crate needs default-features = false and another uses default features, then the default features must be disabled in workspace manifest (eg. [workspace.dependencies] dep = { "0.1", default-features = false }, and enabled in member crate that needs them (eg. dep = { workspace = true, default-features = true }. The other way around will not work.
This should be another check by cargo_sort.py probably.
|
Nice!!! |
353dfa8 to
a04f638
Compare
266b996 to
42975b5
Compare
| flate2.workspace = true | ||
| itertools.workspace = true | ||
| packable = { version = "0.8", default-features = false, features = ["io"] } | ||
| packable = { workspace = true, features = ["io"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| packable = { workspace = true, features = ["io"] } | |
| packable.workspace = true |
"io" feature is enabled in the root.
| num-traits = "0.2.18" | ||
| once_cell.workspace = true | ||
| packable = { version = "0.8", default-features = false, features = ["io"] } | ||
| packable = { workspace = true, features = ["io"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| packable = { workspace = true, features = ["io"] } | |
| packable.workspace = true |
"io" feature is enabled in the root.
| lru.workspace = true | ||
| nonempty.workspace = true | ||
| num-bigint = { version = "0.4", default-features = false, features = ["rand"] } | ||
| num-bigint = { workspace = true, features = ["rand"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "rand" feature really used? I haven't found a place where we generate random BigInts.
| better_any.workspace = true | ||
| fastcrypto.workspace = true | ||
| fastcrypto-vdf.workspace = true | ||
| fastcrypto-vdf = { git = "https://github.com/MystenLabs/fastcrypto", rev = "69d496c71fb37e3d22fe85e5bbfd4256d61422b9", features = ["experimental"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make sense to keep fastcrypto-vdf dependency in the root Cargo.toml close to other fastcrypto deps that also have the same rev? Otherwise fastcrypto deps might go out of sync.
| codespan-reporting.workspace = true | ||
| log.workspace = true | ||
| ouroboros.workspace = true | ||
| ouroboros = "0.17.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update it to "0.18" to avoid duplicates? ouroboros-0.18 is used in typed-store.
| [dependencies] | ||
| # external dependencies | ||
| clap = { version = "4.1.4", features = ["derive"] } | ||
| clap = { workspace = true, features = ["derive"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| clap = { workspace = true, features = ["derive"] } | |
| clap.workspace = true |
"derive" feature is enabled in the root.
| async-trait.workspace = true | ||
| bcs.workspace = true | ||
| clap = { version = "4.1.4", features = ["derive"] } | ||
| clap = { workspace = true, features = ["derive"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| clap = { workspace = true, features = ["derive"] } | |
| clap.workspace = true |
"derive" feature is enabled in the root.
| tonic-prost-build = "0.14" | ||
| tonic-rustls = "0.3.0" | ||
| tower = { version = "0.4.12", features = ["full", "util", "timeout", "load-shed", "limit"] } | ||
| tower = { version = "0.4.12", default-features = false, features = ["full", "util", "timeout", "load-shed", "limit"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need "full" feature? It enables everything, the other features including.
| tower = { version = "0.4", default-features = false, features = ["util"] } | ||
| tracing = { version = "0.1" } | ||
| tokio-util.workspace = true | ||
| tower = { workspace = true, features = ["util"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| tower = { workspace = true, features = ["util"] } | |
| tower.workspace = true |
"util" feature is enabled in the root.
| - If an external dependency is only used by one crate: | ||
| - Removes it from root Cargo.toml (if present) | ||
| - Ensures the crate has the full version spec | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a note how features are handled, it's not trivial after all.
Description of change
This PR adds a new "consolidation mode" to
cargo_sort.py, which will analyze all dependencies across the workspace and:[workspace.dependencies]and updates crates to usepackage.workspace = trueHow the change has been tested