Skip to content

Address all TODO in v27 #318

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

Merged
merged 2 commits into from
Aug 5, 2025
Merged

Conversation

jamillambert
Copy link
Collaborator

@jamillambert jamillambert commented Aug 1, 2025

Go through all the TODO in the v26 types table.

  • Redefine getnodeaddresses: There were return field changes in v27. Redefine the struct and update the reexports. Update the types tables.
  • Run the formatter: Done separately to make it easier to see the changes in patch 1.

DRAFT because on top of #321. This PR is the last 2 patches.

@@ -96,4 +96,6 @@ pub struct NodeAddress {
pub address: Address<NetworkUnchecked>,
Copy link
Member

Choose a reason for hiding this comment

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

Ha, we botched this when added in #191. I wrote some comments on that issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in #321

tcharding
tcharding previously approved these changes Aug 5, 2025
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK cf4694c

@jamillambert jamillambert dismissed tcharding’s stale review August 5, 2025 03:51

The merge-base changed after approval.

tcharding
tcharding previously approved these changes Aug 5, 2025
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK cf4694c

@tcharding
Copy link
Member

Trivial rebase, no conflicts.

@jamillambert jamillambert dismissed tcharding’s stale review August 5, 2025 07:39

The merge-base changed after approval.

The return fields changed in v27.

Redefine the type in v27, update reexports.
Reordering of reexports only.
@jamillambert
Copy link
Collaborator Author

Rebased, no changes

@jamillambert jamillambert marked this pull request as ready for review August 5, 2025 08:29
@jrakibi
Copy link

jrakibi commented Aug 5, 2025

ACK 93ed34f

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 93ed34f

@tcharding tcharding merged commit 5af7bb0 into rust-bitcoin:master Aug 5, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants