Skip to content

ci: fix build-test job with --no-default-features, add miniscript/no-std#1636

Merged
ValuedMammal merged 1 commit intobitcoindevkit:masterfrom
notmandatory:fix/ci_no_default_miniscript_std
Oct 1, 2024
Merged

ci: fix build-test job with --no-default-features, add miniscript/no-std#1636
ValuedMammal merged 1 commit intobitcoindevkit:masterfrom
notmandatory:fix/ci_no_default_miniscript_std

Conversation

@notmandatory
Copy link
Copy Markdown
Member

@notmandatory notmandatory commented Oct 1, 2024

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:

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

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.
@notmandatory notmandatory self-assigned this Oct 1, 2024
@notmandatory notmandatory added the ci label Oct 1, 2024
@notmandatory notmandatory added this to the 1.0.0-beta milestone Oct 1, 2024
Copy link
Copy Markdown
Collaborator

@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 0e80824

@ValuedMammal ValuedMammal merged commit b8746c8 into bitcoindevkit:master Oct 1, 2024
Copy link
Copy Markdown
Collaborator

@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.

post-merge ACK 0e80824

I think this approach also partially unblocks #1018 and #1575, should we move forward with those?

@ValuedMammal ValuedMammal mentioned this pull request Oct 2, 2024
32 tasks
@notmandatory notmandatory deleted the fix/ci_no_default_miniscript_std branch May 26, 2025 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants