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

[Bug]Fix pagination when ApiOptions.StartPage is explicitly set. #2778

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

pkindruk
Copy link
Contributor

Resolves #1955, #2602


Before the change?

  • Following code never returns a list of release and hits API rate limit:
var client = new GitHubClient(new ProductHeaderValue("Test"));
var releases = await client.Repository.Release.GetAll("octokit", "octokit.net",
        new ApiOptions { StartPage = 1, PageSize = 10, PageCount = 2 });
  • Looks like any method prefixed with GetAll* is affected when called with ApiOptions.StartPage != null specified and one of the following is true:
    • ApiOptions.PageCount > 1
    • ApiOptions.PageCount == null

After the change?

  • Example code returns 20 releases as expected

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Sep 22, 2023
@nickfloyd nickfloyd changed the title Fix pagination when ApiOptions.StartPage is explicitly set. [Bug]Fix pagination when ApiOptions.StartPage is explicitly set. Sep 22, 2023
Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for knocking this out @pkindruk ❤️

@nickfloyd nickfloyd merged commit 57a29e9 into octokit:main Sep 22, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

ApiOptions and PageCount
2 participants