Skip to content

Trivial Bindings Updates #2229

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

Conversation

TheBlueMatt
Copy link
Collaborator

One missing constructor and some initial no-exports on the BOLT12 code which I'd like to at least expose the workable parts of (which, sadly, isnt much).

Comment on lines +380 to +381
///
/// This is not exported to bindings users as functions aren't currently mapped.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be on the struct? It only can be created from InvoiceBuilder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well once we fix the builders to work in bindings, we should also add a sign that just takes a private key?

Comment on lines +351 to +352
///
/// This is not exported to bindings users as functions are not yet mapped.
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise.

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.09 🎉

Comparison is base (b734735) 91.55% compared to head (53c48c1) 91.65%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2229      +/-   ##
==========================================
+ Coverage   91.55%   91.65%   +0.09%     
==========================================
  Files         104      104              
  Lines       51749    52184     +435     
  Branches    51749    52184     +435     
==========================================
+ Hits        47379    47827     +448     
+ Misses       4370     4357      -13     
Impacted Files Coverage Δ
lightning-invoice/src/lib.rs 80.02% <ø> (ø)
lightning/src/offers/invoice.rs 90.18% <ø> (ø)
lightning/src/offers/invoice_request.rs 93.40% <ø> (ø)
lightning/src/offers/offer.rs 89.88% <ø> (ø)
lightning/src/offers/parse.rs 93.33% <ø> (ø)
lightning/src/offers/refund.rs 94.36% <ø> (ø)

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TheBlueMatt TheBlueMatt force-pushed the 2023-04-115-bindings-upstream-2 branch from 4c26091 to 53c48c1 Compare April 25, 2023 22:51
@TheBlueMatt
Copy link
Collaborator Author

Added more no-export tags for the types which conflict with other takes (specifically Invoice and the errors). We should fix that in general so that users can import both Invoice types without confusion in their downstream code.

Comment on lines +119 to +120
///
/// This is not exported to bindings users as its name conflicts with the BOLT 11 ParseError type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not exporting this affect using FromStr and TryFrom implementations for Offer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, those are not exported as a result.

@TheBlueMatt TheBlueMatt merged commit cbfff99 into lightningdevkit:main Apr 26, 2023
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.

4 participants