From e556bb89345ebf08980514f1b516c5911ab21d4e Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 4 Jan 2020 14:47:55 +0100 Subject: [PATCH] Adds pagination to Github request. Fixes #3. Unfortunately most of the Github APIs returning collections returns maximum 30 responses. This is documented here https://developer.github.com/v3/#pagination but unfortunately it is not mentioned in the particular APIs that are affected. So when reading https://developer.github.com/v3/pulls/#list-pull-requests it's not at all obvious that pagination is in operation and that you should add another loop through pages. Periodic labeler does not have pagination implemented, therefore the result is that in big repositories with > 30 PRs only the OLDEST prs will be checked (that's what is the sorting order). If all those 30 PRs are properly labelled, then the plugin will stop working and will not pick any new PRs. There is an interesting behaviour of that - if there are unlabelled old PRs, they will get updated, so - in fact - they will disappear from the "oldest" list and possibly some new PRs that will be checked in the new pass. However once you have 30 oldest, properly labelled PRs, the plugin stops working. That's really annoying and difficult to detect problem - especially if your repo does not have a lot of PRs. Pagination is easy. It is described here: https://github.com/google/go-github/#pagination --- main.go | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/main.go b/main.go index d3a900c..8b3080f 100644 --- a/main.go +++ b/main.go @@ -2,9 +2,9 @@ package main import ( "context" - "strings" "os" "path" + "strings" "github.com/golang/glog" "golang.org/x/oauth2" @@ -14,13 +14,13 @@ import ( ) func contains(slice []string, item string) bool { - set := make(map[string]struct{}, len(slice)) - for _, s := range slice { - set[s] = struct{}{} - } + set := make(map[string]struct{}, len(slice)) + for _, s := range slice { + set[s] = struct{}{} + } - _, ok := set[item] - return ok + _, ok := set[item] + return ok } func getCurrentLabels(pr *github.PullRequest) []string { @@ -33,7 +33,7 @@ func getCurrentLabels(pr *github.PullRequest) []string { func containsLabels(expected []string, current []string) bool { for _, e := range expected { - if ! contains(current, e) { + if !contains(current, e) { return false } } @@ -97,18 +97,25 @@ func main() { } opt := &github.PullRequestListOptions{State: "open", Sort: "updated"} - pulls, _, err := client.PullRequests.List(context.Background(), owner, repo, opt) - if err != nil { - glog.Fatal(err) - } - for _, pull := range pulls { - files, _, err := client.PullRequests.ListFiles(context.Background(), owner, repo, *pull.Number, nil) + // get all pages of results + for { + pulls, resp, err := client.PullRequests.List(context.Background(), owner, repo, opt) if err != nil { - glog.Error(err) + glog.Fatal(err) + } + for _, pull := range pulls { + files, _, err := client.PullRequests.ListFiles(context.Background(), owner, repo, *pull.Number, nil) + if err != nil { + glog.Error(err) + } + expectedLabels := matchFiles(labelMatchers, files) + if !containsLabels(expectedLabels, getCurrentLabels(pull)) { + client.Issues.AddLabelsToIssue(context.Background(), owner, repo, *pull.Number, expectedLabels) + } } - expectedLabels := matchFiles(labelMatchers, files) - if ! containsLabels(expectedLabels, getCurrentLabels(pull)) { - client.Issues.AddLabelsToIssue(context.Background(), owner, repo, *pull.Number, expectedLabels) + if resp.NextPage == 0 { + break } + opt.Page = resp.NextPage } }