Skip to content

chore: project restructure#34

Merged
notmandatory merged 1 commit intobitcoindevkit:masterfrom
reez:four
May 15, 2025
Merged

chore: project restructure#34
notmandatory merged 1 commit intobitcoindevkit:masterfrom
reez:four

Conversation

@reez
Copy link
Collaborator

@reez reez commented May 1, 2025

Description

This PR restructures the project, separating core library functionality from FFI bindings. The goal is to support what was needed for PR #32 while still allowing the swift bindings to be built successfully.

Notes to the reviewers

  • created dedicated cktap-ffi crate
  • removed ffi specific code from main library

Changelog notice

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

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

@reez
Copy link
Collaborator Author

reez commented May 2, 2025

@notmandatory @praveenperera

Wanted to get your feedback on what you'd like me to test on this PR to make sure this doesn't break your setups or cause some regression for something I might not be aware of for you.

(If you have general feedback on the PR too definitely let me know, but just wanted to minimize the work you needed to contribute to this PR other than just letting me know what I should test to make sure how you use the library stays the same.)

This was referenced May 4, 2025
@reez reez changed the title (draft)chore: project restructure chore: project restructure May 5, 2025
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 f23819a

Looks good, I verified everything built including the swift. And ran tests with the emulator and the swift test.

One small change I'll make a separate PR for is to remove the unneeded uniffi feature in the cktap-ffi crate since you'll always want it to be on.

@notmandatory notmandatory merged commit 68ded0f into bitcoindevkit:master May 15, 2025
6 checks passed
notmandatory added a commit that referenced this pull request May 15, 2025
6513693 feat: satscard status (Matthew)

Pull request description:

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

  ### Description

  Builds on #34 architecture, adds Satscard `status` command.

  ### Notes to the reviewers

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  ### Changelog notice

  <!-- Notice the release manager should include in the release tag message changelog -->
  <!-- See https://keepachangelog.com/en/1.0.0/ for examples -->

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/notmandatory/rust-cktap/blob/master/CONTRIBUTING.md)
  * [ ] 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

  #### 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

Top commit has no ACKs.

Tree-SHA512: 099908f15ffcd6dd274c0cb70dbb75c5ba8e6dd131e80e53b12a459b9b96112965e99e643831702d19bf53c872e5446e608d9d951cbc3672975ca9cff39b6fa2
notmandatory added a commit that referenced this pull request May 15, 2025
2a67970 fix: swift script unique subdir (Matthew)

Pull request description:

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

  ### Description

  Builds on #34 architecture, and #35 Satscard status command.

  Before this fix, a user might get this error when trying to use 2 swift frameworks created like we do on bdk-ffi bindings.

  bitcoindevkit/bdk-ffi#621

  Example: would prevent a user from using both bdk-swift and cktap-swift in their iOS app potentially.

  ### Notes to the reviewers

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  ### Changelog notice

  <!-- Notice the release manager should include in the release tag message changelog -->
  <!-- See https://keepachangelog.com/en/1.0.0/ for examples -->

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/notmandatory/rust-cktap/blob/master/CONTRIBUTING.md)
  * [ ] 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

  #### 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

ACKs for top commit:
  notmandatory:
    ACK 2a67970

Tree-SHA512: 9dce93e3e803c18a5fd7b20f5cbe9b8b5b22715c46a7f01bd11a78407bba65c57bb8483431ed4a9ce1c72992b8d1e42685128963a2443913d4e61139c46eb9cb
notmandatory added a commit that referenced this pull request May 15, 2025
e133ae6 chore: remove unneeded uniffi feature (Steve Myers)

Pull request description:

  ### Description

  Since #34 we don't need a feature flag for uniffi. It should always be enabled when building the cktap-ffi package.

  ### Checklists

  #### All Submissions:

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

Top commit has no ACKs.

Tree-SHA512: 81f2361d6de06b05ffcb93ba1807efc1ab3d14212e7e88679bb8da77a3b3ab843a5832bb9901e01ab728e6ee61d3680bbb08fa54116724b5446549de613186a1
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