"GET /repos/{owner}/{repo}/commits/{ref}" can paginate the "files" property

[gr2m]

See https://docs.github.com/en/rest/reference/repos#get-a-commit

If there are more than 300 files in the commit diff, the response will include pagination link headers for the remaining files, up to a limit of 3000 files. Each page contains the static commit information, and the only changes are to the file listing.

It's an odd case. Only the "files" key array will be paginated, but unlike the other endpoints that paginate that return an object instead of an array, this one will include lots of other keys. I'm not sure if that case is covered by https://github.com/octokit/plugin-paginate-rest.js/blob/92ed1aab99fe8045fe2c23b21308390471d0c8f5/src/normalize-paginated-list-response.ts

If someone would like to give this a go, I'd be happy to advice. Comment below if you have any questions. The first step would be to add a test to https://github.com/octokit/plugin-paginate-rest.js/blob/master/test/paginate.test.ts, then start a pull request. We can continue the discussion there

[eladchen]

Just read that paragraph myself, and it was evident that pagination ought to be broken given that explanation.

I guess I could try and help, how should we proceed?

[gr2m]

Hi Elad,

first step would be to create a failing test for trying to paginate GET /repos/{owner}/{repo}/commits/{ref}, as mentioned above

If someone would like to give this a go, I'd be happy to advice. Comment below if you have any questions. The first step would be to add a test to https://github.com/octokit/plugin-paginate-rest.js/blob/master/test/paginate.test.ts, then start a pull request. We can continue the discussion there

Would you like to give that a try?

[eladchen]

I can do that. Any idea what the "next" header value is when the files exceed 300?

…

On Sat, Sep 12, 2020, 21:52 Gregor Martynus ***@***.***> wrote: Hi Elad, first step would be to create a failing test for trying to paginate GET /repos/{owner}/{repo}/commits/{ref}, as mentioned above If someone would like to give this a go, I'd be happy to advice. Comment below if you have any questions. The first step would be to add a test to https://github.com/octokit/plugin-paginate-rest.js/blob/master/test/paginate.test.ts, then start a pull request. We can continue the discussion there Would you like to give that a try? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#155 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3V64L5DKWXF7F6AETDCQLSFO7PRANCNFSM4QV45A7A> .

[gr2m]

Good question, I don't know. I think we will have to create a test case to see for ourselves. I assume there will be no next header once we reach 300

[pmauri01]

Was this problem every resolved, we have fond the same issue

[gr2m]

Probably not? It would be a good starting point would be to create or find a commit that changed 300+ files. If it already paginates successfully then we can close this issue. Otherwise we need to record the response headers and create a failing test that replicates the current behavior. Once we have that, we can discuss how to account for this special case