lightningdevkit
Async wallet bump
Codecov Report
Attention: Patch coverage is 90.08621% with 23 lines in your changes missing coverage. Please review.
Project coverage is 90.78%. Comparing base (
35748f6) to head (e7d43db). Report is 16 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3752 +/- ##
==========================================
+ Coverage 89.92% 90.78% +0.86%
==========================================
Files 160 161 +1
Lines 129322 137021 +7699
Branches 129322 137021 +7699
==========================================
+ Hits 116290 124393 +8103
+ Misses 10345 9956 -389
+ Partials 2687 2672 -15
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
👋 The first review has been submitted!
Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.
🔔 1st Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
Basically LGTM, but could you include some details in your commit messages? In general I'd like to be able to read the commit message and understand why a change was done - "Async closures are complicated. Preparatory commit." tells me absolutely nothing.
Yes, commit message on that one was definitely a bit too loose. Added some more detail on why the async closure needed to go.
🔔 2nd Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 3rd Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 4th Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 1st Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 5th Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 2nd Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 6th Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 3rd Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 7th Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 4th Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
🔔 5th Reminder
Hey @TheBlueMatt! This PR has been waiting for your review. Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.
Excuse the delay here, finally got to testing this out with LDK Node.
One request for a minor change, otherwise it seems to work as expected with minmal changes (see https://github.com/tnull/ldk-node/tree/2025-02-upgrade-to-ldk-0.2-with-async-bumper)
Looks good that code. I also found myself doing the _inner thing to wrap it easily into async without too much indent.
As already mentioned in https://github.com/lightningdevkit/rust-lightning/pull/3752#discussion_r2132415654, a lot of the asyncification could be reverted now that the right wrappers are available.
I also isolated the wrappers in their own file.
Reviewers, didn't do fix up commits as the commit structure changed substantially. Thanks for your patience with this PR. Should have realized that the sync wrappers could avoid all the test changes that have now been reverted.
