-
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
Trivial Bindings Updates #1934
Trivial Bindings Updates #1934
Conversation
Codecov ReportBase: 90.78% // Head: 90.76% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1934 +/- ##
==========================================
- Coverage 90.78% 90.76% -0.03%
==========================================
Files 96 96
Lines 50080 50080
Branches 50080 50080
==========================================
- Hits 45464 45453 -11
- Misses 4616 4627 +11
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 at Codecov. |
9ca0fea
to
3784b44
Compare
`ScorerAccountingForInFlightHtlcs` generally stores a `Score` reference generated by calling `LockableScore::lock`, which actually returns an arbitrary `Score`. Given `Score` is implemented directly on lock types, it makes sense to simply hold a fully owned `Score` in `ScorerAccountingForInFlightHtlcs` rather than a mutable reference to one.
Specifically, `OnionMessageContents` is a non-cloneable enum, which isn't stored opaque so we cannot call `&self` methods on it. Because its methods aren't critical to the API for now, we simply no-export them rather than trying to work out an alternative approach.
3784b44
to
f00bc4d
Compare
@@ -118,13 +118,16 @@ pub enum OnionMessageContents<T: CustomOnionMessageContents> { | |||
|
|||
impl<T: CustomOnionMessageContents> OnionMessageContents<T> { | |||
/// Returns the type that was used to decode the message payload. | |||
/// | |||
/// (C-not exported) as methods on non-cloneable enums are not currently exportable |
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.
Does this need a release note somewhere?
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.
We generally haven't release-noted this kinda thing. The nice thing about the comments is they appear in the documentation, but sadly when we do the bindings update after the release they won't show up until the next release :(. Once we get to 0.1 I want to redo our release process to do a rcX
release, then update the bindings, then once that's done do the actual release.
A few trivial things for bindings, but which make sense upstream.