-
Notifications
You must be signed in to change notification settings - Fork 6
CI: add trigger tests workflow on bdk-ffi merges #19
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
CI: add trigger tests workflow on bdk-ffi merges #19
Conversation
There was a problem hiding this 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.
| - name: "Install Rust 1.84.1" | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: 1.84.1 |
There was a problem hiding this comment.
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@v1which should automatically pick up the rust-toolchain.toml file and grab our desired compiler version.
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK db0fe55.
|
First test of the remote trigger for the workflow here: https://github.com/bitcoindevkit/bdk-python/actions/runs/18206142647 |
Description
This PR adds a workflow that handles trigger event
trigger-bdk-python-testwhen 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):
The token's value must be added as repo secret into the bdk-ffi repo.
Checklists
Bugfixes: