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

feat(chain,core)!: move Merge to bdk_core #1625

Merged

Conversation

evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented Sep 26, 2024

Fixes #1624

Description

Moving Merge into bdk_core (from bdk_chain) allows persist crates to only depend on bdk_core.

Changelog notice

  • Move Merge into bdk_core.
  • Remove bdk_chain dependency from bdk_file_store.

Checklists

All Submissions:

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

@evanlinjin evanlinjin self-assigned this Sep 26, 2024
@evanlinjin evanlinjin marked this pull request as ready for review September 26, 2024 09:52
@evanlinjin evanlinjin added dependencies Pull requests that update a dependency file chore labels Sep 26, 2024
@evanlinjin evanlinjin added this to the 1.0.0-beta milestone Sep 26, 2024
Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a4cf905

I tested it with the examples, and they worked fine. It's valid to notice that they still use it from bdk_chain, as it's reexporting it from bdk_core.

Copy link
Contributor

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a4cf905

@LagginTimes
Copy link
Contributor

ACK a4cf905

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a4cf905

@notmandatory notmandatory merged commit e4ee629 into bitcoindevkit:master Oct 1, 2024
21 checks passed
ValuedMammal added a commit that referenced this pull request Oct 1, 2024
…iniscript/no-std

0e80824 ci: fix build-test job with --no-default-features, add miniscript/no-std (Steve Myers)

Pull request description:

  ### Description

  Fixes the CI `build-test` job with `--no-default-features` by also adding `--features miniscript/no-std`.

  Until `rust-miniscript` removes the `no-std` feature we need to enable it when `--no-default-features` is used to build `bdk_wallet` or the whole workspace. See also the `check-no-std` job which does the same plus enables the `bdk_chain/hashbrown` feature which is also needed to build `bdk_wallet` with `--no-default-features` but is already enabled when building the whole workspace.

  ### Notes to the reviewers

  I think we didn't catch this on #1625 because the CI job names changed and I didn't update the branch merge requirements. Another possibility is it was passing because of cached build artifacts which I removed last night when I was trying to troubleshoot something else. I've updated the required CI jobs that need to pass before allowing a PR to be merged to `master` to include the ones with `--no-default-features --features bdk_chain/hashbrown` in the name.

  ### Checklists

  #### All Submissions:

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

ACKs for top commit:
  ValuedMammal:
    ACK 0e80824

Tree-SHA512: 5da486b7fd988575b6f9c06eb108a183b47c74d58fd451453d77b53ad26f58890ee605f4a154922688dc348bc5a3c413dcd9128fd4831d8923c64a33aa4a951c
@ValuedMammal ValuedMammal mentioned this pull request Oct 2, 2024
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore dependencies Pull requests that update a dependency file
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Move Merge trait to bdk_core.
5 participants