-
Notifications
You must be signed in to change notification settings - Fork 698
chore: disable all unused dependencies' default-features #6367
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
This reverts commit 915fcf5.
mutants = "0.0.3" | ||
rstest = "0.17.0" | ||
[package.metadata.cargo-machete] | ||
ignored = ["slog"] |
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.
slog
is needed when we use the various info!
, debug!
, etc. macros. cargo machete
is not so good with macros though and marks it as unused (same story for all others [package.metadata.cargo-machete]
in the various files)
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.
the dependencies on this file were not changed because #6371 will move this crate out of the workspace
# This dependency is used for the multiversion integration tests which live behind the build-v3-1-0-0-13 feature flag | ||
libsigner_v3_1_0_0_13 = { package = "libsigner", git = "https://github.com/stacks-network/stacks-core.git", rev = "8a79aaa7df0f13dfc5ab0d0d0bcb8201c90bcba2", optional = true } | ||
signer_v3_1_0_0_13 = { package = "stacks-signer", git = "https://github.com/stacks-network/stacks-core.git", rev = "8a79aaa7df0f13dfc5ab0d0d0bcb8201c90bcba2", optional = true, features = ["testing", "default"] } | ||
stacks_common_v3_1_00_13 = { package = "stacks-common", git = "https://github.com/stacks-network/stacks-core.git", rev = "8a79aaa7df0f13dfc5ab0d0d0bcb8201c90bcba2", optional = true, features = ["testing", "default"] } | ||
stacks_v3_1_00_13 = { package = "stackslib", git = "https://github.com/stacks-network/stacks-core.git", rev = "8a79aaa7df0f13dfc5ab0d0d0bcb8201c90bcba2", optional = true, features = ["testing", "default"] } |
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.
I left these here because I am not sure if these will be removed in the future and were a temporary test?
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.
This is a question for @jferrant I think.
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.
the changes in this file were needed because mio::tcp::
was a deprecated module, internally exposing mio::net::
. The "forwarding" of the module was feature gated behind #[cfg(feature = "with-deprecated")]
(included by default)
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.
Heh, we just need to rip out mio
altogether at some point (#3886).
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.
This change is fine, btw.
readme = "README.md" | ||
edition = "2021" | ||
resolver = "2" | ||
resolver = "3" |
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.
Here and elsewhere, resolver
only needs to be specified in the workspace root
@@ -56,3 +38,29 @@ devtools = [] | |||
rollback_value_check = [] | |||
disable-costs = [] | |||
wasm-web = ["stacks_common/wasm-web"] | |||
|
|||
[dependencies] | |||
hashbrown.workspace = true |
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.
Since we’re reviewing the overall workspace approach, I’ll mention a small style point here.
Functionally <dependency>.workspace = true
is fine and my comment is purely about consistency.
Could we consider always using the <dependency> = { workspace = true }
form?
Even though it’s slightly more verbose, this keeps the format uniform across the workspace, makes the dependency name stand out more clearly, and simplifies future edits if we need to add more fields.
libsigner = { path = "./libsigner", default-features = false } | ||
libstackerdb = { path = "./libstackerdb", default-features = false } | ||
pox-locking = { path = "./pox-locking", default-features = false } | ||
stacks = { package = "stackslib", path = "./stackslib", default-features = false } # Alias for stackslib |
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.
Not for this PR, but I’m wondering if we could remove the stacks
alias and just use stackslib
directly.
This would require updating use statements across the codebase from use stacks::...
to use stackslib::...
but it shouldn’t be a critical change.
The main benefit would be improved clarity and consistency.
Description
centralized all dependencies
common to 2+ cratesinto the workspace'sCargo.toml
.Updated workspace crates to use workspace = true.
Set
default-features = false
for all crates, enabling only explicitly required features.Unified dependency versions across crates; upgraded
chrono
instacks-node
to matchstacks-common
(0.4.19
→0.4.41
).Removed unused crates (
cargo machete --with-metadata
) and added exceptions for required ones, e.g.:Applied
cargo sort -g -w
to format all Cargo.toml files.Moved to
resolver = "3"
Applicable issues
Additional info (benefits, drawbacks, caveats)
Checklist
docs/rpc/openapi.yaml
andrpc-endpoints.md
for v2 endpoints,event-dispatcher.md
for new events)clarity-benchmarking
repobitcoin-tests.yml