go-gerrit icon indicating copy to clipboard operation
go-gerrit copied to clipboard

Fix handling of Skip and Start parameters for pagination

Open andygrunwald opened this issue 3 years ago • 2 comments

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

  1. We aim to provide a Migration path
  2. Pagination is a common and important functionality, especially on bigger instances
  3. 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.

andygrunwald avatar Sep 19 '21 12:09 andygrunwald

@dmitshur @opalmer I would love to get your quick feedback on this.

andygrunwald avatar Sep 19 '21 12:09 andygrunwald

@dmitshur If you find some spare time, it would be great to get your pair of eyes and a bit of feedback. Thanks mate!

andygrunwald avatar May 27 '22 17:05 andygrunwald