Skip to content
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

Make updateLocalFundingStatus method return Either #2602

Merged
merged 3 commits into from
Feb 20, 2023
Merged

Conversation

pm47
Copy link
Member

@pm47 pm47 commented Feb 20, 2023

After calling this method, we perform actions at several places that only make sense if the correct behavior happened. Instead of assuming things went ok, we use proper typing and make the result explicit.

This is a follow-up to #2598.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
After calling this method, we perform actions at several places that
only make sense if the correct behavior happened. Instead of assuming
things went ok, we use proper typing and make the result explicit.
@pm47 pm47 requested a review from t-bast February 20, 2023 15:47
t-bast
t-bast previously approved these changes Feb 20, 2023
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2023

Codecov Report

Merging #2602 (b832f48) into master (a3c6029) will decrease coverage by 0.09%.
The diff coverage is 71.27%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #2602      +/-   ##
==========================================
- Coverage   85.74%   85.66%   -0.09%     
==========================================
  Files         211      211              
  Lines       16909    16913       +4     
  Branches      714      728      +14     
==========================================
- Hits        14499    14488      -11     
- Misses       2410     2425      +15     
Impacted Files Coverage Δ
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 87.16% <55.81%> (-0.23%) ⬇️
...in/scala/fr/acinq/eclair/channel/Commitments.scala 96.41% <71.42%> (-0.21%) ⬇️
...inq/eclair/channel/fsm/ChannelOpenDualFunded.scala 88.00% <80.95%> (-0.51%) ⬇️
...q/eclair/channel/fsm/ChannelOpenSingleFunded.scala 93.44% <87.50%> (-0.78%) ⬇️
...inq/eclair/channel/fsm/CommonFundingHandlers.scala 94.87% <100.00%> (-0.13%) ⬇️
...q/eclair/channel/publish/ReplaceableTxFunder.scala 85.45% <0.00%> (-3.04%) ⬇️
...main/scala/fr/acinq/eclair/router/Validation.scala 93.80% <0.00%> (-1.24%) ⬇️
...scala/fr/acinq/eclair/router/BalanceEstimate.scala 98.91% <0.00%> (-1.09%) ⬇️
...inq/eclair/channel/fund/InteractiveTxBuilder.scala 89.12% <0.00%> (-0.54%) ⬇️
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 85.41% <0.00%> (+1.38%) ⬆️
... and 1 more

@pm47 pm47 changed the title Make `updateLocalFundingStatus method return Either Make updateLocalFundingStatus method return Either Feb 20, 2023
@pm47 pm47 merged commit df590d8 into master Feb 20, 2023
@pm47 pm47 deleted the update-status-either branch February 20, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants