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

lightningd: allow 'style' 'legacy' for clboss compat (with deprecated_apis) #5120

Merged
merged 2 commits into from
Mar 24, 2022

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Mar 23, 2022

We keep the parameter around for future use (e.g. PTLC support?), but clboss
master still sets 'style' to "legacy". Accept, but ignore it.

Reported-by: grubman on #c-lightning (IRC)
Changelog-None: change introduced this release

@rustyrussell rustyrussell added this to the v0.11 milestone Mar 23, 2022
@rustyrussell rustyrussell requested a review from ZmnSCPxj March 23, 2022 00:02
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

LGTM, only some minor in the style convention.

In addition, is it better to add in this case a Changelog-Deprecated?

lightningd/pay.c Outdated Show resolved Hide resolved
@rustyrussell
Copy link
Contributor Author

LGTM, only some minor in the style convention.

In addition, is it better to add in this case a Changelog-Deprecated?

We already documented that we removed legacy, but you're right.

…_apis).

We keep the parameter around for future use (e.g. PTLC support?), but clboss
master still sets 'style' to "legacy".  Accept, but ignore it.

Reported-by: grubman on #c-lightning (IRC)
Changelog-Deprecated: JSON-RPC: `sendpay` `route` argument `style` "legacy" (don't use it at all, we ignore it now and always use "tlv" anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Well, it is leaking a bool for the command duration, but that's probably OK.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@vincenzopalazzo
Copy link
Contributor

We already documented that we removed legacy, but you're right.

I put on my to-do list that may be necessary to remove this remove legacy line in the Changelog generation.

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ack c4f84a1

@cdecker
Copy link
Member

cdecker commented Mar 23, 2022

ACK c4f84a1

@rustyrussell rustyrussell merged commit f95c952 into ElementsProject:master Mar 24, 2022
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.

3 participants