Skip to content
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

Clean up dependency structure #1611

Open
ValuedMammal opened this issue Sep 13, 2024 · 7 comments
Open

Clean up dependency structure #1611

ValuedMammal opened this issue Sep 13, 2024 · 7 comments
Labels
dependencies Pull requests that update a dependency file
Milestone

Comments

@ValuedMammal
Copy link
Contributor

ValuedMammal commented Sep 13, 2024

Can we switch the file store and testenv crates to depend on bdk_core now instead of bdk_chain to help organize and reduce complexity of the overall dependency graph?

@ValuedMammal ValuedMammal added the dependencies Pull requests that update a dependency file label Sep 13, 2024
@oleonardolima
Copy link
Contributor

I think it'll be hard to do it, as we'll mostly already depend on bdk_testenv which already depends on bdk_chain (just wondering about it for now 🤔)

@evanlinjin
Copy link
Member

@ValuedMammal how would this result in cyclic dependencies? One can't depend on a crate's tests?

@notmandatory
Copy link
Member

notmandatory commented Sep 15, 2024

I suggested this issue since it seems weird that bdk_core has even a test dependency on bdk_chain. It looks like the only reason for the dependency is that the bdk_core doc examples depend on bdk_chain. Is there any reason not to re-write the doc examples or move them to the chain crate?

@oleonardolima
Copy link
Contributor

I suggested this issue since it seems weird that bdk_core has even a test dependency on bdk_chain. It looks like the only reason for the dependency is that the bdk_core doc examples depend on bdk_chain. Is there any reason not to re-write the doc examples or move them to the chain crate?

The test usage in block_id! is being moved to bdk_testenv at #1612, so it won't be an issue anymore. Also, it's weird it depends on bdk_chain::BlockId instead of crate::BlockId 🤔.

I think it's fine to either rewrite the doc examples or move to bdk_chain.

@ValuedMammal ValuedMammal changed the title [core] Remove bdk_chain from dev-dependencies Clean up dependency structure Sep 26, 2024
@ValuedMammal
Copy link
Contributor Author

I'll repurpose this issue into a general discussion of what the structure of the repo's inter-crate dependencies should look like.

@ValuedMammal
Copy link
Contributor Author

I also noticed that bdk_esplora's "std" feature still enables bdk_chain/std even though the idea of #1155 was to drop the dependency on bdk_chain from the chain source crates.

[features]
default = ["std", "async-https", "blocking-https-rustls"]
std = ["bdk_chain/std", "miniscript?/std"]

@notmandatory notmandatory added this to BDK Sep 26, 2024
@notmandatory notmandatory moved this to Discussion in BDK Sep 26, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Sep 26, 2024
@notmandatory
Copy link
Member

Should we also bump major versions from 0.x to 1.x for bdk crates:
bdk_core
bdk_chain
bdk_electrum
bdk_esplora

I'm not sure these are ready to be 1.x and can stay as 0.x:
bdk_bitcoind_rpc
bdk_file_store
bdk_testenv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
Status: Discussion
Development

No branches or pull requests

4 participants