-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: add paging flags for list commands that support paging #297
Conversation
|
||
func (pp *PageParameters) ConfigureFlags(fs *pflag.FlagSet) { | ||
fs.IntVar(&pp.size, "size", 100, "Number of entries to receive at most.") | ||
fs.IntVar(&pp.number, "page", 0, "Page number to calculate first item to receive. Page numbers start from `1`.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These could also be --page-size
and --page-number
to be more descriptive, but those are quite verbose 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As they naturally are available only on list
commands, these shorter ones seem fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was also thinking between --size
and --limit
. Go SDK uses size, but there that is used as field of Page
struct. Maybe limit would clearer without that context 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed --size
→ --limit
|
||
func (pp *PageParameters) ConfigureFlags(fs *pflag.FlagSet) { | ||
fs.IntVar(&pp.size, "size", 100, "Number of entries to receive at most.") | ||
fs.IntVar(&pp.number, "page", 0, "Page number to calculate first item to receive. Page numbers start from `1`.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As they naturally are available only on list
commands, these shorter ones seem fine to me.
This also increases the default page size from 10 to 100.
This also increases the default page size from 10 to 100.