Skip to content

Switch to proc macros module Kyoto#690

Merged
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
rustaceanrob:udl-to-macros-3-8
Mar 11, 2025
Merged

Switch to proc macros module Kyoto#690
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
rustaceanrob:udl-to-macros-3-8

Conversation

@rustaceanrob
Copy link
Copy Markdown
Collaborator

@rustaceanrob rustaceanrob commented Mar 8, 2025

Description

Migrates all kyoto related types to proc macros. The remote enum NodeState remains exported by the UDL as it would require a repeated definition in the kyoto module.

Changelog notice

  • Migrate kyoto module to proc macros

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

@rustaceanrob rustaceanrob marked this pull request as ready for review March 8, 2025 19:39
@rustaceanrob rustaceanrob changed the title Switch to proc macros Switch to proc macros module Kyoto Mar 9, 2025
@thunderbiscuit
Copy link
Copy Markdown
Member

I was hoping we'd be able to do a few smaller ones first to get everyone on board and familiar with that transition, but it turns out that it's hard to do it one little piece at a time, because the UDL is connected to it all and you can't have a UDL that doesn't parse on its own, and you can't declare both a proc macro and a UDL. So for example you can't just pull out the FeeRate type as a single migration, because it's being returned and used in other types.

It looks like the clients are maybe some of the easier parts of the codebase to migrate first... thanks for doing this.

@rustaceanrob
Copy link
Copy Markdown
Collaborator Author

So for example you can't just pull out the FeeRate type as a single migration, because it's being returned and used in other types.

Interesting. If you want to change a type to proc macros, all instances where it is used and returned must also be proc macros? If that is the case then a module-by-module refactor will probably have to go down.

I was just doing this since the earlier I nipped it in the butt for Kyoto the easier it would be to add new types/methods without horrendous code churn.

Btw, I looked in IntelliJ at the JVM build and the doc strings are used when you define them above the types/methods in this way.

Copy link
Copy Markdown
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 f46f743. The live Kyoto tests pass locally.

@thunderbiscuit thunderbiscuit merged commit f46f743 into bitcoindevkit:master Mar 11, 2025
25 checks passed
@rustaceanrob rustaceanrob deleted the udl-to-macros-3-8 branch March 11, 2025 18:57
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