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

Remove payLnInvoice helper #192

Merged
merged 3 commits into from
Oct 24, 2024
Merged

Remove payLnInvoice helper #192

merged 3 commits into from
Oct 24, 2024

Conversation

Egge21M
Copy link
Collaborator

@Egge21M Egge21M commented Oct 23, 2024

Description

This PR removes the payLnInvoice helper function. Instead people should just use createMeltQuote and meltProofs to pay Lightning invoices.

Changes

  • Removes payLnInvoice

PR Tasks

  • Open PR
  • run npm run test --> no failing unit tests
  • run npm run format

@Egge21M Egge21M added the v2 This issue will be solved in the v2 major release label Oct 23, 2024
@callebtc
Copy link
Contributor

Impossible to review due to history.

@Egge21M Egge21M changed the base branch from development to fees-and-coinselection October 23, 2024 15:11
@Egge21M
Copy link
Collaborator Author

Egge21M commented Oct 23, 2024

Impossible to review due to history.

Changed target to fees-and-coinselection for clear history until that is ready to merge

@gandlafbtc
Copy link
Collaborator

looks good, merging it into calle's PR

@gandlafbtc gandlafbtc merged commit 398787b into fees-and-coinselection Oct 24, 2024
4 checks passed
@Egge21M
Copy link
Collaborator Author

Egge21M commented Oct 24, 2024

looks good, merging it into calle's PR

Thanks! Should probably have marked it as draft until the fee PR is merged.
I think we should not merge into a branch that is under review, as reviewers will now see these changes in calle's PR as well, although they already have been reviewed.

@Egge21M Egge21M linked an issue Oct 24, 2024 that may be closed by this pull request
Discuss: deprecate payLnInvoice and payLnInvoiceWithToken from lib #157
Closed
@gandlafbtc
Copy link
Collaborator

Agree 😀 reviewed the PR, saw ready to merge and pulled the trigger. But yeah, let's not do that in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
v2 This issue will be solved in the v2 major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Discuss: deprecate payLnInvoice and payLnInvoiceWithToken from lib
3 participants
@Egge21M @callebtc @gandlafbtc