search

andygrunwald / go-gerrit

Go client/library for Gerrit Code Review
https://godoc.org/github.com/andygrunwald/go-gerrit
MIT License
96 stars 40 forks source link

Fix handling of Skip and Start parameters for pagination #98

Open andygrunwald opened 3 years ago

andygrunwald commented 3 years ago

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 ...

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 commented 3 years ago

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

andygrunwald commented 2 years ago

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