Go-github: Some List* endpoints don't take ListOptions (but they should).

[dmitshur]

Most (but not all) list endpoints support pagination, and take opt *ListOptions as the last parameter (or another specific Options type that embeds ListOptions).

Some List* endpoints do not take a *ListOptions or equivalent parameter, and therefore it's not possible to perform pagination.

It's a breaking API change, but we should fix it sooner rather than later.

One such case was the PullRequestsService.ListReviews method, reported by @sminnee in #635.

Additional cases I found just by looking briefly at the code include PullRequestsService.ListReviewComments and AuthorizationsService.ListGrants. There are probably more, I haven't done an exhaustive search.

The task is twofold:

1. Identify all such problematic endpoints and list them here.
2. Send a PR similar to #635 to fix them and bump libraryVersion up by one.

Also, we should be more mindful of this potential pitfall when doing reviews in the future. /cc @gmlewis

[sminnee]

Ok so I went through all the "List" functions and these are the ones that don't have options parameters.

These do not have pagination

• [ ] activity.go: ListFeeds
• [ ] gitignore.go: List
• [ ] misc.go: ListEmojis
• [ ] misc.go: ListServiceHooks
• [ ] licenses.go: List
• [ ] repos.go: ListLanguages
• [ ] repos_stats.go: ListContributorsStats
• [ ] repos_stats.go: ListCommitActivity
• [ ] repos_stats.go: ListCodeFrequency
• [ ] repos_stats.go: ListParticipation
• [ ] repos_stats.go: ListPunchCard
• [ ] repos_traffic.go: ListTrafficReferrers
• [ ] repos_traffic.go: ListTrafficPaths
• [ ] repos.go: ListRequiredStatusChecksContexts

These do have pagination

• [ ] authorizations.go: ListGrants
• [ ] gists.go: ListCommits
• [ ] pulls_reviewers.go: ListReviewers
• [ ] pulls_reviews.go: ListReviews
• [ ] pulls_reviews.go: ListReviewComments
• [ ] repos_pages.go: ListPagesBuilds
• [ ] users.go: ListInvitations

_(Edited by @shurcooL to include latest updates, so this serves as the canonical list for this issue.)_

[sminnee]

The indication from https://developer.github.com/v3/#pagination is that every list-based feed will paginate when more than 30 items are returned. So I would think that adding ListOptions to all of these is the best idea.

[dmitshur]

I haven't yet validated whether the API provides pagination on each of these.

It's important that we do this.

For example, ActivityService.ListFeeds in activity.go should _not_ have ListOptions, because the endpoint does not support pagination:

https://developer.github.com/v3/activity/feeds/#list-feeds

Notice that it returns a single JSON object, not array, and there's no "Link" headers with pagination under "Status: 200 OK" text:

It's just an endpoint that happens to have "List" in its name.

Endpoints that actually support pagination look like this:

Notice they there are Link headers with pagination under "Status: 200 OK", and it returns a JSON array.

[sminnee]

I've had a look into this: I think that the Link heading is only returned if there are more than 1 page of results — it is not returned on a single-page response that supports pagination in cases where there are more items.

So, I've found a better way to check is to put "?page=2" on the end of the URL and see if the output changes.

[sminnee]

OK: I've updated my previous comment break the list out into pagination / non.

[sminnee]

I've now expanded the scope of #635 to be a complete fix to this. You should be able to close this issue once that is merged.

[dmitshur]

I don't have a place to test this but I assume it's got pagation.

• [ ] repos_pages.go: ListPagesBuilds

It's documented to have it, yes.

Ok so I went through all the "List" functions and these are the ones that don't have options parameters.

I've verified, and everything looks good, except "licenses.go: List".

It's documented not to have pagination support:

When I did:

$ curl -H 'Accept: application/vnd.github.drax-preview+json' -i https://api.github.com/licenses | grep Link

The Link header wasn't set.

However, I saw that doing:

$ curl -H 'Accept: application/vnd.github.drax-preview+json' -i https://api.github.com/licenses?page=2

$ curl -H 'Accept: application/vnd.github.drax-preview+json' -i https://api.github.com/licenses?per_page=1

Actually works and sets Link header then.

I don't think we should add it. It's likely that the endpoint supports pagination unintentionally, which could be a bug that GitHub will fix. There's only ~8 licenses, chances are they won't grow exponentially, so there's not going to be need for pagination.

All others look good. Thanks for doing the work!

[dmitshur]

Actually, UsersService.ListInvitations is documented not to have pagination either:

https://developer.github.com/v3/repos/invitations/#list-a-users-repository-invitations

But given the nature of repository invitations, I expect that endpoint should absolutely support pagination. I suspect it's a bug in their API documentation.

Edit: Further evidence it should have pagination is that List invitations for a repository, a very similar endpoint, is documented to support pagination.

Would you want to send them an email at [email protected] to get clarification and get it fixed?

[sminnee]

OK I've emailed github about this.

Looks like licenses was an clerical error on my part, I'll remove from the PR.

[dmitshur]

We got a reply from GitHub, see https://github.com/google/go-github/pull/635#issuecomment-300743066.

It confirms that UsersService.ListInvitations endpoint should also support pagination, as we expected.

[dmitshur]

This issue was fully resolved by #635.