Skip to content

Migrate Connection type to procedural macros#731

Merged
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
thunderbiscuit:feature/proc-macros
Apr 22, 2025
Merged

Migrate Connection type to procedural macros#731
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
thunderbiscuit:feature/proc-macros

Conversation

@thunderbiscuit
Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit commented Apr 22, 2025

Migration of the Connection type.

Docs pulled directly from the rusqlite crate: https://docs.rs/rusqlite/0.31.0/rusqlite/struct.Connection.html#method.open

One thing to note is that with proc macros, all methods in an impl block will be exported if that block is marked with the #uniffi::export annotation, meaning that if we want to keep some methods private/out of the public API for the bindings (for example the Connection::get_store()), they simply need to be brought into a separate impl block.

@thunderbiscuit thunderbiscuit requested a review from reez April 22, 2025 15:05
@thunderbiscuit thunderbiscuit changed the title refactor: migrate Connection type to procedural macros Migrate Connection type to procedural macros Apr 22, 2025
@reez
Copy link
Copy Markdown
Collaborator

reez commented Apr 22, 2025

One thing to note is that with proc macros, all methods in an impl block will be exported if that block is marked with the #uniffi::export annotation, meaning that if we want to keep some methods private/out of the public API for the bindings (for example the Connection::get_store()), they simply need to be brought into a separate impl block.

THANKS for calling this out, I was confused but then I looked at this and it cleared it up for me!

Copy link
Copy Markdown
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

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

ACK 2fd7aa1

@thunderbiscuit thunderbiscuit merged commit 2fd7aa1 into bitcoindevkit:master Apr 22, 2025
26 checks passed
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