-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add EventSenderExt
to work with Wallet
directly
#88
Add EventSenderExt
to work with Wallet
directly
#88
Conversation
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.
I made comments, otherwise this looks good.
src/lib.rs
Outdated
fn add_revealed_scripts<'a>( | ||
&'a self, | ||
wallet: &'a bdk_wallet::Wallet, | ||
) -> FutureResult<(), kyoto::ClientError>; |
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.
fyi I just learned that rust 1.83 will issue a warning for an "elided named lifetime"
warning: elided lifetime has a name
--> src/lib.rs:502:22
|
499 | fn add_revealed_scripts<'a>(
| -- lifetime `'a` declared here
...
502 | ) -> FutureResult<(), kyoto::ClientError>;
| ^ this elided lifetime gets resolved as `'a`
|
= note: `#[warn(elided_named_lifetimes)]` on by default
src/lib.rs
Outdated
} | ||
Ok(()) | ||
} | ||
Box::pin(_add_revealed(&self, wallet)) |
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.
Also caught by CI but apparently we allow warnings. We could change clippy in CI to cargo clippy --all-targets -- -Dwarnings
Box::pin(_add_revealed(&self, wallet)) | |
Box::pin(_add_revealed(self, wallet)) |
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.
Yeah we can follow up with that patch to CI
bafa77d
to
4e54637
Compare
Updated with patches to those comments. |
9fc9485
to
ef79790
Compare
ef79790
to
c182d50
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.
ACK c182d50
No description provided.