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

Fee calculation and presentation #294

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Fee calculation and presentation #294

merged 1 commit into from
Oct 25, 2023

Conversation

ademar111190
Copy link
Contributor

@ademar111190 ademar111190 commented Jul 3, 2023 •
edited
Loading

@ademar111190 ademar111190 self-assigned this Jul 3, 2023
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 5 times, most recently from 0ad5495 to 2408dd6 Compare July 7, 2023 12:20
roeierez
roeierez requested changes Jul 9, 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.

Starting to shape up! It is hard to review though with all the formatting changes.

libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/breez_services.rs Outdated Show resolved Hide resolved
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch from c10b688 to 35216e7 Compare July 10, 2023 12:27
@ademar111190 ademar111190 mentioned this pull request Jul 10, 2023
Merged
@ademar111190
Copy link
Contributor Author

Starting to shape up! It is hard to review though with all the formatting changes.

Thanks, please don't hesitate to give feedback about the implementation.

Regarding the changes, I hope these #307 and #297 helps to reduce it from this PR

@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch from 7fa04f9 to 6e9c505 Compare July 11, 2023 14:44
@ademar111190 ademar111190 changed the base branch from main to run-cargo-fmt July 11, 2023 14:44
roeierez
roeierez requested changes Jul 13, 2023
View reviewed changes
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-bindings/src/breez_sdk.udl Outdated Show resolved Hide resolved
Base automatically changed from run-cargo-fmt to main July 14, 2023 13:12
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 2 times, most recently from 3c9aec2 to a4d9122 Compare July 18, 2023 11:43
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 3 times, most recently from a652203 to d726df7 Compare August 2, 2023 12:36
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 6 times, most recently from 7f9e596 to 2652a0d Compare August 14, 2023 12:21
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 3 times, most recently from 74bc6b3 to 3b19745 Compare August 17, 2023 13:15
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 4 times, most recently from 72944af to 6b218f2 Compare September 14, 2023 12:48
@ademar111190 ademar111190 changed the title WIP: fee calculation and presentation Fee calculation and presentation Sep 14, 2023
@ademar111190 ademar111190 marked this pull request as ready for review September 14, 2023 12:49
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch from 6b218f2 to 082df39 Compare September 14, 2023 13:16
roeierez
roeierez requested changes Sep 14, 2023
View reviewed changes
libs/sdk-core/src/models.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Outdated Show resolved Hide resolved
libs/sdk-bindings/src/breez_sdk.udl Outdated Show resolved Hide resolved
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch from 082df39 to 9b3fad4 Compare September 15, 2023 12:24
roeierez
roeierez requested changes Sep 17, 2023
View reviewed changes
libs/sdk-core/src/breez_services.rs Outdated Show resolved Hide resolved
libs/sdk-bindings/src/breez_sdk.udl Outdated Show resolved Hide resolved
libs/sdk-bindings/src/breez_sdk.udl Outdated Show resolved Hide resolved
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 3 times, most recently from de7513c to 4ef741c Compare September 18, 2023 12:43
roeierez
roeierez requested changes Sep 18, 2023
View reviewed changes
libs/sdk-bindings/src/breez_sdk.udl Outdated Show resolved Hide resolved
libs/sdk-core/src/greenlight/node_api.rs Show resolved Hide resolved
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch from 4ef741c to 8492cc5 Compare September 18, 2023 13:46
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch 2 times, most recently from 57b517d to 1e38098 Compare October 9, 2023 12:23
@roeierez
Copy link
Member

@ademar111190 this looks good and almost ready IMO.
I am missing the React Native generated files.

@dangeross
Copy link
Collaborator

Here's some info to get you started with the RN codegen:

https://github.com/breez/breez-sdk/blob/main/libs/sdk-react-native/DEVELOPING.md

@ademar111190
Copy link
Contributor Author

thanks for the link @dangeross
not sure why but I had a lot of problems installing things, In the end I managed to do it by disabling ipv6 on my machine

@ademar111190
Copy link
Contributor Author

@roeierez gen files added; let me know if it is missing something

@roeierez roeierez mentioned this pull request Oct 17, 2023
Closed
@ademar111190 ademar111190 force-pushed the fee-calculation-and-presentation branch from 427ea16 to b5316b4 Compare October 24, 2023 12:33
roeierez
roeierez approved these changes Oct 25, 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!
Pease make sure the RN generated code is up to date with the latest generator before merging.

@ademar111190
Copy link
Contributor Author

@roeierez yeap; I did use it on my last rebase.
@dangeross can you double check if there is something wrong? What I can see that is correct but my knowledge in rn is almost 0

dangeross
dangeross approved these changes Oct 25, 2023
View reviewed changes
Copy link
Collaborator

@dangeross dangeross left a comment

Choose a reason for hiding this comment

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

Looks great!

@ademar111190 ademar111190 merged commit 89dd16a into main Oct 25, 2023
5 checks passed
@ademar111190 ademar111190 deleted the fee-calculation-and-presentation branch October 25, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ok300 ok300 ok300 left review comments

@roeierez roeierez roeierez approved these changes

@dangeross dangeross dangeross approved these changes

@erdemyerebasmaz erdemyerebasmaz Awaiting requested review from erdemyerebasmaz

Assignees

@ademar111190 ademar111190

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ademar111190 @roeierez @dangeross @ok300