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

lncli: change listinvoices reversed flag to paginate-forwards #4003

Merged

Conversation

bitromortac
Copy link
Collaborator

The listinvoices --reversed flag is changed to --paginate-forwards to fix #3986.

Currently, if one likes to start to paginate from the beginning of the invoices history
one has to set --reversed=False, which is not very intuitive. Otherwise the
--reversed flag has no effect at all, as it is set by default. The downside of the change
is that it diverts from the grpc API, where the reversed flag still is present.

If this change is wanted I would apply it also to the payments listing in PR #3960.

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 🥇
I think a boolean flag with default value true defeats the purpose of a flag. This makes more sense to me.
I'm fine with it being called differently on gRPC.

cmd/lncli/commands.go Outdated Show resolved Hide resolved
@guggero guggero requested a review from carlaKC February 14, 2020 07:54
@guggero guggero added this to the 0.10.0 milestone Feb 14, 2020
@guggero guggero added cli Related to the command line interface UI Related to the User Interface v0.10 labels Feb 14, 2020
Copy link
Collaborator

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

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

Looks good to me, I think it's fine having the rpc and cli have different values. This is much more intuitive, nice PR 🔥

@bitromortac bitromortac force-pushed the listinvoices-reversed-flag branch from ba5025d to f47e34a Compare February 19, 2020 07:25
This commit renames the `reversed` pagination flag to
`paginate-forwards`, which is off by default. In order to
access older invoices one can set the paginate-forwards flag,
which is more intuitive than setting the reversed flag to false.
@bitromortac bitromortac force-pushed the listinvoices-reversed-flag branch from f47e34a to 926d4ba Compare February 25, 2020 10:34
@Roasbeef Roasbeef merged commit 237ef9c into lightningnetwork:master Feb 26, 2020
@bitromortac bitromortac deleted the listinvoices-reversed-flag branch April 17, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command line interface UI Related to the User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

listinvoices: reversed flag has no effect
4 participants