allow to set the requested page on all endpoints #586
Conversation
At the moment it's not possible to request another page on all endpoints (search for example). With this change the requested `page` can be set the same way like the `per_page`.
There was a problem hiding this comment.
Many endpoints allow to specify a $param which allow pagination. See https://github.com/KnpLabs/php-github-api/blob/master/lib/Github/Api/CurrentUser.php#L179
lib/Github/Api/AbstractApi.php
Outdated
| * | ||
| * @var null|int | ||
| */ | ||
| protected $page; |
There was a problem hiding this comment.
The same why $perPage isn't private - it should be extendable.
There was a problem hiding this comment.
I would make $perPage private to if it didn't break BC.
It is easier to change from private to protected than the other way around.
There was a problem hiding this comment.
ping @Gummibeer, please change this property to private
|
@Nyholm yes, but by far not all - and I think there are two ways: define the page like in this PR or allow the |
|
I do like the idea of solving this for all apis. Do all endpoints support pagination? |
|
Every endpoint that returns more than one entry. |
|
@Einenlum any chance to get it merged and released soon? |
|
ping @acrobat |
|
Only a minor comment left, but after that it looks good to merge! |
|
Sorry wrong button :( |
|
Done |
|
Yes indeed! Last comments were fixed so merging! Thanks @Gummibeer! |
At the moment it's not possible to request another page on all endpoints (search for example). With this change the requested
pagecan be set the same way like theper_page.