Skip to content
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

Include optional params parameter for Commits compare method #1053

Merged
merged 1 commit into from
Mar 8, 2022

Conversation

mountiny
Copy link
Contributor

The Github compare API limits the response to only 250 commits. I have been testing this library and I have not been able to paginate the compare call without using the params array and adding the per_page key. Without it, I always got only 250 commits in the diff.

When the $params is included, I have managed to get paginate all the commits from the Github API.

Unfortunately, I cannot easily use the all call with the since parameter because I only have the base and head available.

I think this change would be valuable in general, it should not break any existing behaviour and it will enable pagination for compare calls.

Thank you very much for the review and considering this change, it would help a lot to our team.

With the current API, we cannot paginate compare basehead function call. The Github API limits the response to only 250 commits so for larger diffs we have no option.
@mountiny
Copy link
Contributor Author

mountiny commented Mar 8, 2022

@acrobat any thoughts on this one? We would find this update quite handy and it should not break any existing code.

@acrobat acrobat merged commit ad546e2 into KnpLabs:master Mar 8, 2022
@acrobat
Copy link
Collaborator

acrobat commented Mar 8, 2022

Thanks @mountiny, sorry for the delay! And congrats on your first contribution! 🎉

@mountiny
Copy link
Contributor Author

mountiny commented Mar 8, 2022

No worries! Thakn you!

@mountiny
Copy link
Contributor Author

@acrobat Any estimates for next release with this PR included? Thanks!

@acrobat
Copy link
Collaborator

acrobat commented Mar 24, 2022

@mountiny I've just released a new version, see https://github.com/KnpLabs/php-github-api/releases/tag/v3.6.0

@mountiny
Copy link
Contributor Author

Legend! Thanks! @acrobat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants