Repository navigation
[Feature Request] Allow page size option (per_page) in listPullRequestFiles call #408
Description
The listPullRequests call allows an optional second parameter for options that include setting the page size for for a request. GitHub allows per_page to also be sent along for the call wrapped by listPullRequestFiles but this library doesn't allow options to be passed.
I'm happy to submit a PR but want to know:
- If you're cool with this
- How you'd like me to add this functionality without breaking any existing uses of the library that rely on the second parameter being a callback. implement a
listPullRequestFilesWithOptions?
Activity
clayreimann commentedon Dec 19, 2016
I for one am cool with this idea. But, are you sure we don't just fetch all pages by default? Many of our methods fetch all of the pages behind the scenes.
We probably want to re-think how we implement paged methods in general.
rausanka commentedon Dec 24, 2016
Based on https://github.com/github-tools/github/blob/master/lib/Repository.js, it seems that the only Repository call that requests all pages is
listProjects. Also, it doesn't look like the use of_requestAllPagesinlistProjectstakes advantage of using theper_pageoption to increase page sizes from 30 to 100.Given this, it seems like there are a few possible improvements (not necessarily mutually exclusive):
per_pageto 100 to reduce necessary requests to get all pageslistPullRequestFilesfor getting individual pages as before and a newlistAllPullRequestFilesfor recursively getting all files across all pages).rausanka commentedon Jul 29, 2017
Been thinking about this more recently and I think we should definitely do #2 above and I'd prefer #3 over #1 as large repositories might have many pages (e.g. at QuanticMind we're creating ~300 PRs/month) that will quickly get you into GitHub API rate limiting hell. Thoughts?