-
Notifications
You must be signed in to change notification settings - Fork 385
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
Small Cleanups Discovered during Bindings for 0.0.111 #1714
Small Cleanups Discovered during Bindings for 0.0.111 #1714
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1714 +/- ##
==========================================
- Coverage 90.84% 90.80% -0.05%
==========================================
Files 86 86
Lines 46448 46451 +3
Branches 46448 46451 +3
==========================================
- Hits 42198 42180 -18
- Misses 4250 4271 +21
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
89275cd
to
7f9df8d
Compare
7f9df8d
to
53aeeeb
Compare
ACK 53aeeeb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, feel free to squash fixups.
Now that `{Signed,}RawInvoice` implement the std `Hash` trait, having a method called `hash` is ambiguous. Instead, we rename the `hash` methods `signed_hash` to make it clear that the hash is the one used for the purpose of signing the invoice.
The C bindings generator isn't capable of figuring out if a blanket impl applies in a given context, and instead opts to always write out any relevant impl's for a trait. Thus, blanket impls should be disabled when building with `#[cfg(c_bindings)]`.
The bindings generator is pretty naive in its generic resolution and doesn't like `where` clauses for bounds that are simple traits. This should eventually change, but for now its simplest to just inline the relevant generic bounds.
The "helpful" type aliases don't make sense for C bindings as all generics are concretized anyway.
I'm not sure why rustc didn't complain about the unused parameter or why we're allowed to get away without explicitly bounding the `Sign` in the `KeysInterface`, but the current code requires all `BlindedPath` construction to explicitly turbofish an unused type.
53aeeeb
to
be7107f
Compare
Tweaked one more doc comment as suggested and squashed fixups. |
When updating the sample, I found out that the |
Done. |
Ugh, sorry, was missing an |
105809e
to
0c6040d
Compare
Note that `SimpleArcPeerHandler` is also updated to not wrap `IgnoringMessageHandler` in an `Arc`, as `IgnoringMessageHandler` is already zero-sized.
0c6040d
to
b78359a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, 'j, 'k, SD, M, T, F, C, L
👀
Really only the first is a practical change for Rust users, the others are just here because otherwise they'd be on the 0.0.111-bindings branch by themselves.