Skip to content

Trivial #3246 followups. - #3959

Merged
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
TheBlueMatt:2025-07-3246-followups
Jul 25, 2025
Merged

Trivial #3246 followups.#3959
TheBlueMatt merged 2 commits into
lightningdevkit:mainfrom
TheBlueMatt:2025-07-3246-followups

Conversation

@TheBlueMatt

@TheBlueMattTheBlueMatt commented Jul 24, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@ldk-reviews-bot

ldk-reviews-bot commented Jul 24, 2025•
edited
Loading

Copy link
Copy Markdown

👋 I see @tankyleo was un-assigned.
If you'd like another reviewer assignment, please click here.

@TheBlueMattTheBlueMatt mentioned this pull request Jul 24, 2025
Merged
@codecov

codecovBot commented Jul 24, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.93%. Comparing base (55baa15) to head (86c186a).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #3959 +/- ##
==========================================
- Coverage 88.94% 88.93% -0.02% 
==========================================
Files 173 173 Lines 123794 123794 Branches 123794 123794 ==========================================
- Hits 110114 110094 -20 - Misses 11227 11242 +15 - Partials 2453 2458 +5
FlagCoverage Δ
fuzzing22.21% <ø> (ø)
tests88.76% <ø> (-0.02%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jkczyz
jkczyz reviewed Jul 24, 2025
View reviewed changes
Comment threadlightning/src/ln/channelmanager.rs
@jkczyz
jkczyz removed the request for review from tankyleoJuly 24, 2025 21:55
@TheBlueMatt
TheBlueMattforce-pushed the 2025-07-3246-followups branch from 974ffeb to 1f4d837CompareJuly 24, 2025 22:22
@TheBlueMatt

TheBlueMatt commented Jul 24, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Oops, fixed, no changes just comit reordering.

shaavan
shaavan reviewed Jul 25, 2025
View reviewed changes

@shaavanshaavan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Just one small nit

Comment threadlightning/src/ln/channelmanager.rs Outdated
@TheBlueMatt
Be more pedantically correct in offer/refund builder documentation
86c186a
Instead of making vague reference to "those docs", indicate exactly
which docs we're suggesting the user go hunt for. Also, for variant
methods, just say "see the original method for more details" rather
than trying to spell out all the things that exist in the
documentation for the other method. This avoids things getting
stale and doesn't reduce the information users have.
@TheBlueMatt
TheBlueMattforce-pushed the 2025-07-3246-followups branch from 1f4d837 to 86c186aCompareJuly 25, 2025 13:44
@TheBlueMatt

TheBlueMatt commented Jul 25, 2025

Copy link
Copy Markdown
CollaboratorAuthor

oops bad rebase sorry, fixed.

jkczyz
jkczyz approved these changes Jul 25, 2025
View reviewed changes

@jkczyzjkczyz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CI failure looks like test flakiness

@TheBlueMatt

TheBlueMatt commented Jul 25, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Yea, I'm not quite sure what those issues are. Some DNS issue (though DNS lookups tested manually on the server complete ~instantly)...

@TheBlueMatt
TheBlueMatt merged commit 5ceb625 into lightningdevkit:mainJul 25, 2025
26 of 28 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkczyzjkczyzjkczyz approved these changes
+1 more reviewer
@shaavanshaavanshaavan left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TheBlueMatt@ldk-reviews-bot@jkczyz@shaavan