-
-
Notifications
You must be signed in to change notification settings - Fork 109
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 additional column ordering to keysetpagination #640
Conversation
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.
Nice, this already looks very good 👍
Should we maybe allow multiple additional fields? We might not use it, but then it will be easy to do once it comes up.
@zepatrik I'd like to postpone the support for multiple fields for now. I've spent too much time on this already, and that would just complicate it, IMO. I'll add some more tests, but then it should be good to go. |
var _ PageTokenConstructor = NewMapPageToken | ||
var _ PageTokenConstructor = NewStringPageToken | ||
|
||
type PageTokenConstructor func(string) (PageToken, error) |
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.
type PageTokenConstructor func(string) (PageToken, error) | |
type PageTokenConstructor = func(string) (PageToken, error) |
A type alias allows to just pass a value without ever needing to do type assertions.
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.
Ah, nice. Don't think this changes much in the current PR. Or did you have something specific in mind?
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.
Nice 🎉
cc @zepatrik
Related Issue or Design Document
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
Further comments