go-gerrit
go-gerrit copied to clipboard
Fix handling of Skip and Start parameters for pagination
The way how we handle pagination is not optimal, partially broken, and sometimes even inconsistent with the current (v3.4.1) Gerrit REST API docs.
Not optimal Parameters are defined as strings in some Options. It should be an integer.
Partially broken
In some calls, you can set a Skip
and Start
parameters.
On Gerrit Server Side this is causing a (we assume) undefined behavior.
However, the call with both parameters s
and start
set will not succeed.
Inconsistent with the current (v3.4.1) Gerrit REST API docs In the Gerrit docs, the setting is sometimes named Start. Sometimes Skip. On go-gerrit side, we use kinda both and something the other version per call.
This Pull Request is ...
- Fixing and unifying the Start/Skip behavior
- Being consistent with the Gerrit REST API docs
- Adding examples on how to use the functionality
Breaking change
This PR is introducing a breaking change. I believe it is worth doing it, because
- We aim to provide a Migration path
- Pagination is a common and important functionality, especially on bigger instances
- The breaking change doesn't require a huge amount of modification
Special notice
This PR is a follow-up from the work of @michaeldorner in https://github.com/andygrunwald/go-gerrit/pull/48.
After the merge
I aim to create a new library release including a migration guide for people who are adopting this library.
@dmitshur @opalmer I would love to get your quick feedback on this.
@dmitshur If you find some spare time, it would be great to get your pair of eyes and a bit of feedback. Thanks mate!