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

Merge bitcoindevkit/bdk-cli#99: The Great Reset #107

Closed
wants to merge 1 commit into from

Conversation

waterst0ne
Copy link
Contributor

292dd1e Fix repl mode command parsing (Steve Myers)
073f1c3 Update with review comments (rajarshimaitra)
4e8f830 revert author list change (rajarshimaitra)
b09c405 Remove base64 dependency (rajarshimaitra)
1e70ff9 Refactor everything (rajarshimaitra)

Pull request description:

Description

This is a massive refactoring PR that changes the whole structure of the crate. Previously it was written like a library
to be used to create the bdk-cli app. But eventually the crate itself became the app. This PR attempts to remove the remaining
lib like patterns in the code, and make it a pure binary crate.

This makes the code more modular and makes it look like a typical binary rust crate.

There was no real good way to structure the change into separate commits, so I made one single big one.. The best way to review is to look at the final structure of the code itself, not the change set.

The crate has following modules now

  • main : The main app runtime
  • commands: Includes all the structopt commands used by bdk-cli.
  • handlers: Include all the command handlers used buy the app.
  • utils: Include all the utility and helper functions
  • Backend : Defines the backend node process, and its related methods. (This will be filled more with Unleash the power of Bitcoin Core into bdk-cli #92).

Apart from the structure changes there are few other changes that took place

  • Almost all of the previous doc comments are removed. As they were written to use bdk-cli as a lib. Instead new structopts "comments" are added to describe the app functionality better. As a result the app --help commands are more elaborate and descriptive now. I have also removed few redundant description messages used before, that would mess up the help comments. And as a by product it solves Bug in help doc #93.

  • bdk is updated to v0.19.0

  • bdk-reserves is updated with current version pointing to bdk v0.19.0.

  • Default database is now sqlite.

Overall I think I managed not to break anything.

Currently this change will remove most of the previous documentation on the crate. But those aren't useful to context of bdk-cli after this change.. My proposal would be reproduce the README instructions itself in doc.rs landing page.

We also need to update the README to reflect these changes.. I will open that up in a separate PR.

I also haven't updated changelog yet.. Not sure yet how to describe the change in short.. Will do that once this is almost finalized..

Notes to the reviewers

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

ACKs for top commit:
notmandatory:
ACK 292dd1e

Tree-SHA512: 895d8088bf93a481fd776e2ac5fe85926f13b7b4535f17b9edd3c0363a89dc3689e28c6e13dbcac3970bc00e3ff206f402e94406f3b3688c9e4a7f9d31b20e40

Description

Notes to the reviewers

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature
  • I've updated CHANGELOG.md

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

292dd1e Fix repl mode command parsing (Steve Myers)
073f1c3 Update with review comments (rajarshimaitra)
4e8f830 revert author list change (rajarshimaitra)
b09c405 Remove base64 dependency (rajarshimaitra)
1e70ff9 Refactor everything (rajarshimaitra)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  This is a massive refactoring PR that changes the whole structure of the crate. Previously it was written like a library
  to be used to create the bdk-cli app. But eventually the crate itself became the app. This PR attempts to remove the remaining
  lib like patterns in the code, and make it a pure binary crate.

  This makes the code more modular and makes it look like a typical binary rust crate.

  There was no real good way to structure the change into separate commits, so I made one single big one.. The best way to review is to look at the final structure of the code itself, not the change set.

  The crate has following modules now
   -  `main` : The main app runtime
   -  `commands`: Includes all the structopt commands used by bdk-cli.
   -  `handlers`: Include all the command handlers used buy the app.
   -  `utils`: Include all the utility and helper functions
   - `Backend` : Defines the backend node process, and its related methods. (This will be filled more with bitcoindevkit#92).

  Apart from the structure changes there are few other changes that took place
   - Almost all of the previous doc comments are removed. As they were written to use bdk-cli as a lib. Instead new structopts "comments" are added to describe the app functionality better. As a result the app `--help` commands are more elaborate and descriptive now. I have also removed few redundant description messages used before, that would mess up the help comments. And as a by product it solves bitcoindevkit#93.

   - bdk is updated to v0.19.0

   - bdk-reserves is updated with current version pointing to bdk v0.19.0.

   - Default database is now sqlite.

   Overall I think I managed not to break anything.

   Currently this change will remove most of the previous documentation on the crate. But those aren't useful to context of bdk-cli after this change.. My proposal would be reproduce the README instructions itself in doc.rs landing page.

   We also need to update the README to reflect these changes.. I will open that up in a separate PR.

   I also haven't updated changelog yet.. Not sure yet how to describe the change in short.. Will do that once this is almost finalized..

  ### Notes to the reviewers

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [ ] I've added docs for the new feature
  * [ ] I've updated `CHANGELOG.md`

ACKs for top commit:
  notmandatory:
    ACK 292dd1e

Tree-SHA512: 895d8088bf93a481fd776e2ac5fe85926f13b7b4535f17b9edd3c0363a89dc3689e28c6e13dbcac3970bc00e3ff206f402e94406f3b3688c9e4a7f9d31b20e40
@waterst0ne waterst0ne closed this Jul 7, 2022
@waterst0ne waterst0ne deleted the Add_addData branch July 7, 2022 23:24
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.

2 participants