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

Should use pagination to find more than (default) 30 files on a PR #19

Open
ruudk opened this issue Oct 5, 2023 · 3 comments
Open

Should use pagination to find more than (default) 30 files on a PR #19

ruudk opened this issue Oct 5, 2023 · 3 comments

Comments

@ruudk
Copy link

ruudk commented Oct 5, 2023

I noticed that the action doesn't work on a PR with 1100 files.

I saw the following on the documentation:

List pull requests files
Note: Responses include a maximum of 3000 files. The paginated response returns 30 files per page by default.
source

I believe the action should use pagination as described here:
https://octokit.github.io/rest.js/v20#pagination

const { data: prFiles } = await octokit.rest.pulls.listFiles({
owner,
repo,
pull_number: prNumber,
mediaType: {
format: 'diff'
}
});

ruudk added a commit to ruudk/github-action-todo-commenter that referenced this issue Oct 5, 2023
Fixes gkampitakis#19

This increases the page size from the default 30 to the maximum of 100.

It then uses `octokit.paginate` to automatically fetch all pages.
@gkampitakis
Copy link
Owner

🤔 if it returns 3000 files then why it didn't work with 1100 files? Was there maybe another issue?

@ruudk
Copy link
Author

ruudk commented Oct 6, 2023

It did not work on my PR, because the action only saw 30 files. As this is the default page size, and no pagination was used.

It has nothing to do with my PR having 1100 files and the upper limit of 3000 (paginatable) files.

@gkampitakis
Copy link
Owner

Sorry misread your description. Thanks for the pr. I have commented there with the updated test and one comment.

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 a pull request may close this issue.

2 participants