Skip to content

Conversation

@mg-twentyone
Copy link
Contributor

Description

This PR adds a workflow that handles trigger event trigger-bdk-python-test when new commits are merged into the master branch of bdk-ffi.

Fixes #17

Notes to the reviewers

This PR is linked to bdk-ffi PR.

TODO (Repository owner):

  • create Personal Access Token (classic) with permissions for workflow and repo (i guess we could include only public_repo).
    The token's value must be added as repo secret into the bdk-ffi repo.

Checklists

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

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Looking good. Just a few comments/ideas.

Comment on lines 54 to 57
- name: "Install Rust 1.84.1"
uses: actions-rs/toolchain@v1
with:
toolchain: 1.84.1
Copy link
Member

Choose a reason for hiding this comment

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

This is an old and deprecated action. You can instead use

- name: "Set up Rust"
  uses: actions-rust-lang/setup-rust-toolchain@v1

which should automatically pick up the rust-toolchain.toml file and grab our desired compiler version.

Copy link
Contributor Author

@mg-twentyone mg-twentyone Oct 2, 2025

Choose a reason for hiding this comment

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

This is a valid alternative. However, at the moment the library has an issue in handling the rust_src_dir flag, which we need to specify the path where to find the toml file (inside the submodule).
I created a PR to fix it. While waiting for the fix, we can use it setting a static toolchain version value.

- name: "Build wheel"
run: python -m build --wheel --verbose

- name: "Upload artifact test"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we need to upload these artifacts for the jobs. Did you have a use for them in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My initial idea was to reuse the built wheel distributions, sharing them with the publishing workflow that could exclusively handle the upload.
However, currently, the publish workflow executes the build (I assume the publication is managed manually), so right now it’s useless, we can eliminate the artifacts’ upload from this workflow.

@thunderbiscuit
Copy link
Member

Once this is merged we can start testing it by triggering the workflow manually, so that's cool. We don't even need the PR on bdk-ffi to be in to start using it.

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

ACK db0fe55.

@thunderbiscuit thunderbiscuit merged commit db0fe55 into bitcoindevkit:master Oct 2, 2025
17 checks passed
@thunderbiscuit
Copy link
Member

First test of the remote trigger for the workflow here: https://github.com/bitcoindevkit/bdk-python/actions/runs/18206142647

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.

Add CI trigger scripts to run tests when new commits are added to bdk-ffi

2 participants