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.

Sign up for GitHub

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

Jump to bottom

switch to use amountMsat for upcomming sdk release. #688

Merged
merged 1 commit into from
Oct 23, 2023
Merged

switch to use amountMsat for upcomming sdk release. #688

merged 1 commit into from
Oct 23, 2023

Conversation

ubbabeck
Copy link
Contributor

@ubbabeck ubbabeck commented Oct 20, 2023 •
edited
Loading

@ubbabeck ubbabeck self-assigned this Oct 20, 2023
@ubbabeck ubbabeck changed the title [WIP]switch to use amountMsat for upcomming sdk release. switch to use amountMsat for upcomming sdk release. Oct 23, 2023
@ubbabeck ubbabeck marked this pull request as ready for review October 23, 2023 08:31
erdemyerebasmaz
erdemyerebasmaz requested changes Oct 23, 2023
View reviewed changes
Copy link
Collaborator

@erdemyerebasmaz erdemyerebasmaz left a comment

Choose a reason for hiding this comment

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

Looks solid!

Have some NIT comments.

I think most of the payment related functions on AccountBloc should take their new Request object as parameter instead of multiple variables.

lib/bloc/account/account_bloc.dart Outdated Show resolved Hide resolved
lib/bloc/account/credentials_manager.dart Outdated Show resolved Hide resolved
lib/bloc/refund/refund_bloc.dart Outdated Show resolved Hide resolved
lib/bloc/reverse_swap/reverse_swap_bloc.dart Outdated Show resolved Hide resolved
lib/bloc/sweep/sweep_bloc.dart Outdated Show resolved Hide resolved
lib/routes/lnurl/payment/lnurl_payment_handler.dart Outdated Show resolved Hide resolved
@ubbabeck ubbabeck force-pushed the ubba-Msat branch 2 times, most recently from 3a2f988 to 81d4566 Compare October 23, 2023 11:41
erdemyerebasmaz
erdemyerebasmaz approved these changes Oct 23, 2023
View reviewed changes
Copy link
Collaborator

@erdemyerebasmaz erdemyerebasmaz left a comment

Choose a reason for hiding this comment

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

LGTM! 👏

CI changes should be reverted after

gets merged and we're ready to go!

roeierez
roeierez approved these changes Oct 23, 2023
View reviewed changes
Copy link
Member

@roeierez roeierez left a comment

Choose a reason for hiding this comment

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

LGTM

@ubbabeck ubbabeck merged commit 50dd4d0 into main Oct 23, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roeierez roeierez roeierez approved these changes

@erdemyerebasmaz erdemyerebasmaz erdemyerebasmaz approved these changes

Assignees

@ubbabeck ubbabeck

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ubbabeck @erdemyerebasmaz @roeierez