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

Split CLIs #24

Closed
wants to merge 14 commits into from
Closed

Split CLIs #24

wants to merge 14 commits into from

Conversation

ppoliani
Copy link
Contributor

@ppoliani ppoliani commented Jan 27, 2024

Fixes #13

The following changes were applied:

  1. Create a Cargo workspace
  2. Split the current code base into three distinct crates
  • zkbitcoin-core: Lib crate that contains common code used by the CLI binaris
  • zkbtc: The CLI used by users
  • zkbtc-admin: CLI used for admin tasks

@ppoliani ppoliani changed the title [WIP] Split CLIs Split CLIs Jan 27, 2024
@mimoo
Copy link
Contributor

mimoo commented Jan 28, 2024

Hey! Thanks for the PR. I'm a bit mixed about merging it because I think it's doing too much and at this point we haven't really talked about a reorganization of crates or why we should do this. Do you mind just having a PR that creates an additional zkbtc-admin bin?

@ppoliani
Copy link
Contributor Author

ppoliani commented Jan 28, 2024

@mimoo I could add just one more crate, we would still have to create a Cargo workspace the way it's done in this PR. The other thing is that we might end up with cyclic dependencies if we have just two crates. This is is pretty much a common crate project organization where common code just goes to a separate crate to avoid potential cyclic dependencies.

In fact, the changes in this PR are pretty simple. The current code stayed in the zkbitcoin-core crate (basically renamed the current crate) except for the bin file which was split into two separate crates.

@mimoo
Copy link
Contributor

mimoo commented Jan 28, 2024

but why add one more crate? We can have different binaries in the current bin/ directory while keeping the same organization :o

@ppoliani
Copy link
Contributor Author

Yes we can keep things in one single crate. Mixing things would violate SRP. While this might not look very important at the moment, it might bite us back once the codebase grows, and at that point it might be more challenging to split the code inot disitnct logical units.

In general, following the workspace solution is a common practise when building bigger Rust projects (with the potential to grow). I do though understand where you're coming from; so I can understand if you don't want to make any such changes at this point.

@mimoo
Copy link
Contributor

mimoo commented Jan 30, 2024

let's keep everything in a single crate for now if you don't mind, sorry for the push back!

@ppoliani
Copy link
Contributor Author

let's keep everything in a single crate for now if you don't mind, sorry for the push back!

No worries at all :) Will do the split in the same crate

@ppoliani ppoliani mentioned this pull request Jan 30, 2024
@ppoliani
Copy link
Contributor Author

@mimoo closing this in favour of #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move orchestartor/node commands in a different CLI
2 participants