From 145b29d70d574177905eef277a8def5ca2f63b5a Mon Sep 17 00:00:00 2001 From: Dhananjay Mishra Date: Sun, 8 Feb 2026 14:00:58 +0000 Subject: [PATCH 1/4] Replace UserListOptions.ListOptions with UserListOptions.PerPage --- github/github-iterators.go | 31 -------------- github/github-iterators_test.go | 72 --------------------------------- github/users.go | 4 +- github/users_test.go | 14 +++---- 4 files changed, 6 insertions(+), 115 deletions(-) diff --git a/github/github-iterators.go b/github/github-iterators.go index 6183e71f904..691b5ee10ce 100644 --- a/github/github-iterators.go +++ b/github/github-iterators.go @@ -4179,37 +4179,6 @@ func (s *TeamsService) ListUserTeamsIter(ctx context.Context, opts *ListOptions) } } -// ListAllIter returns an iterator that paginates through all results of ListAll. -func (s *UsersService) ListAllIter(ctx context.Context, opts *UserListOptions) iter.Seq2[*User, error] { - return func(yield func(*User, error) bool) { - // Create a copy of opts to avoid mutating the caller's struct - if opts == nil { - opts = &UserListOptions{} - } else { - opts = Ptr(*opts) - } - - for { - items, resp, err := s.ListAll(ctx, opts) - if err != nil { - yield(nil, err) - return - } - - for _, item := range items { - if !yield(item, nil) { - return - } - } - - if resp.NextPage == 0 { - break - } - opts.ListOptions.Page = resp.NextPage - } - } -} - // ListBlockedUsersIter returns an iterator that paginates through all results of ListBlockedUsers. func (s *UsersService) ListBlockedUsersIter(ctx context.Context, opts *ListOptions) iter.Seq2[*User, error] { return func(yield func(*User, error) bool) { diff --git a/github/github-iterators_test.go b/github/github-iterators_test.go index c7735e6e17d..fa5197ccf22 100644 --- a/github/github-iterators_test.go +++ b/github/github-iterators_test.go @@ -9663,78 +9663,6 @@ func TestTeamsService_ListUserTeamsIter(t *testing.T) { } } -func TestUsersService_ListAllIter(t *testing.T) { - t.Parallel() - client, mux, _ := setup(t) - var callNum int - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - callNum++ - switch callNum { - case 1: - w.Header().Set("Link", `; rel="next"`) - fmt.Fprint(w, `[{},{},{}]`) - case 2: - fmt.Fprint(w, `[{},{},{},{}]`) - case 3: - fmt.Fprint(w, `[{},{}]`) - case 4: - w.WriteHeader(http.StatusNotFound) - case 5: - fmt.Fprint(w, `[{},{}]`) - } - }) - - iter := client.Users.ListAllIter(t.Context(), nil) - var gotItems int - for _, err := range iter { - gotItems++ - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - } - if want := 7; gotItems != want { - t.Errorf("client.Users.ListAllIter call 1 got %v items; want %v", gotItems, want) - } - - opts := &UserListOptions{} - iter = client.Users.ListAllIter(t.Context(), opts) - gotItems = 0 - for _, err := range iter { - gotItems++ - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - } - if want := 2; gotItems != want { - t.Errorf("client.Users.ListAllIter call 2 got %v items; want %v", gotItems, want) - } - - iter = client.Users.ListAllIter(t.Context(), nil) - gotItems = 0 - for _, err := range iter { - gotItems++ - if err == nil { - t.Error("expected error; got nil") - } - } - if gotItems != 1 { - t.Errorf("client.Users.ListAllIter call 3 got %v items; want 1 (an error)", gotItems) - } - - iter = client.Users.ListAllIter(t.Context(), nil) - gotItems = 0 - iter(func(item *User, err error) bool { - gotItems++ - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - return false - }) - if gotItems != 1 { - t.Errorf("client.Users.ListAllIter call 4 got %v items; want 1 (an error)", gotItems) - } -} - func TestUsersService_ListBlockedUsersIter(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/users.go b/github/users.go index 542e7da7026..471b385b4f3 100644 --- a/github/users.go +++ b/github/users.go @@ -212,9 +212,7 @@ type UserListOptions struct { Since int64 `url:"since,omitempty"` // Note: Pagination is powered exclusively by the Since parameter, - // ListOptions.Page has no effect. - // ListOptions.PerPage controls an undocumented GitHub API parameter. - ListOptions + PerPage int `url:"per_page,omitempty"` } // ListAll lists all GitHub users. diff --git a/github/users_test.go b/github/users_test.go index a9523b2f30b..28168315761 100644 --- a/github/users_test.go +++ b/github/users_test.go @@ -326,11 +326,11 @@ func TestUsersService_ListAll(t *testing.T) { mux.HandleFunc("/users", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testFormValues(t, r, values{"since": "1", "page": "2"}) + testFormValues(t, r, values{"since": "1", "per_page": "30"}) fmt.Fprint(w, `[{"id":2}]`) }) - opt := &UserListOptions{1, ListOptions{Page: 2}} + opt := &UserListOptions{Since: 1, PerPage: 30} ctx := t.Context() users, _, err := client.Users.ListAll(ctx, opt) if err != nil { @@ -498,17 +498,13 @@ func TestUserListOptions_Marshal(t *testing.T) { testJSONMarshal(t, &UserListOptions{}, "{}") u := &UserListOptions{ - Since: int64(1900), - ListOptions: ListOptions{ - Page: int(1), - PerPage: int(10), - }, + Since: int64(1900), + PerPage: 30, } want := `{ "since" : 1900, - "page": 1, - "perPage": 10 + "perPage": 30 }` testJSONMarshal(t, u, want) From d3c000063ecd07ee528491b705bb3bca72afd774 Mon Sep 17 00:00:00 2001 From: Dhananjay Mishra Date: Sun, 8 Feb 2026 14:12:12 +0000 Subject: [PATCH 2/4] minor fix --- github/github-accessors.go | 8 ++++++++ github/github-accessors_test.go | 11 +++++++++++ github/users.go | 2 +- github/users_test.go | 4 ++-- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 078bc7b7b40..2a214c07ca5 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -31454,6 +31454,14 @@ func (u *UserLDAPMapping) GetURL() string { return *u.URL } +// GetPerPage returns the PerPage field if it's non-nil, zero value otherwise. +func (u *UserListOptions) GetPerPage() int { + if u == nil || u.PerPage == nil { + return 0 + } + return *u.PerPage +} + // GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. func (u *UserMigration) GetCreatedAt() string { if u == nil || u.CreatedAt == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 93efd460073..be6cc7dfda2 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -40617,6 +40617,17 @@ func TestUserLDAPMapping_GetURL(tt *testing.T) { u.GetURL() } +func TestUserListOptions_GetPerPage(tt *testing.T) { + tt.Parallel() + var zeroValue int + u := &UserListOptions{PerPage: &zeroValue} + u.GetPerPage() + u = &UserListOptions{} + u.GetPerPage() + u = nil + u.GetPerPage() +} + func TestUserMigration_GetCreatedAt(tt *testing.T) { tt.Parallel() var zeroValue string diff --git a/github/users.go b/github/users.go index 471b385b4f3..2b2d8f8028b 100644 --- a/github/users.go +++ b/github/users.go @@ -212,7 +212,7 @@ type UserListOptions struct { Since int64 `url:"since,omitempty"` // Note: Pagination is powered exclusively by the Since parameter, - PerPage int `url:"per_page,omitempty"` + PerPage *int `url:"per_page,omitempty"` } // ListAll lists all GitHub users. diff --git a/github/users_test.go b/github/users_test.go index 28168315761..30548845a87 100644 --- a/github/users_test.go +++ b/github/users_test.go @@ -330,7 +330,7 @@ func TestUsersService_ListAll(t *testing.T) { fmt.Fprint(w, `[{"id":2}]`) }) - opt := &UserListOptions{Since: 1, PerPage: 30} + opt := &UserListOptions{Since: 1, PerPage: Ptr(30)} ctx := t.Context() users, _, err := client.Users.ListAll(ctx, opt) if err != nil { @@ -499,7 +499,7 @@ func TestUserListOptions_Marshal(t *testing.T) { u := &UserListOptions{ Since: int64(1900), - PerPage: 30, + PerPage: Ptr(30), } want := `{ From 67f837cb157cb4e868fbe1729f842248f35832c1 Mon Sep 17 00:00:00 2001 From: Dhananjay Mishra Date: Sun, 8 Feb 2026 14:21:19 +0000 Subject: [PATCH 3/4] feedback --- github/github-accessors.go | 8 ++++++++ github/github-accessors_test.go | 11 +++++++++++ github/users.go | 2 +- github/users_test.go | 4 ++-- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 2a214c07ca5..27c88d51c39 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -31462,6 +31462,14 @@ func (u *UserListOptions) GetPerPage() int { return *u.PerPage } +// GetSince returns the Since field if it's non-nil, zero value otherwise. +func (u *UserListOptions) GetSince() int64 { + if u == nil || u.Since == nil { + return 0 + } + return *u.Since +} + // GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. func (u *UserMigration) GetCreatedAt() string { if u == nil || u.CreatedAt == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index be6cc7dfda2..835f310e0b7 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -40628,6 +40628,17 @@ func TestUserListOptions_GetPerPage(tt *testing.T) { u.GetPerPage() } +func TestUserListOptions_GetSince(tt *testing.T) { + tt.Parallel() + var zeroValue int64 + u := &UserListOptions{Since: &zeroValue} + u.GetSince() + u = &UserListOptions{} + u.GetSince() + u = nil + u.GetSince() +} + func TestUserMigration_GetCreatedAt(tt *testing.T) { tt.Parallel() var zeroValue string diff --git a/github/users.go b/github/users.go index 2b2d8f8028b..31fe4f3f9a6 100644 --- a/github/users.go +++ b/github/users.go @@ -209,7 +209,7 @@ func (s *UsersService) GetHovercard(ctx context.Context, user string, opts *Hove // method. type UserListOptions struct { // ID of the last user seen - Since int64 `url:"since,omitempty"` + Since *int64 `url:"since,omitempty"` // Note: Pagination is powered exclusively by the Since parameter, PerPage *int `url:"per_page,omitempty"` diff --git a/github/users_test.go b/github/users_test.go index 30548845a87..23a00bb7e06 100644 --- a/github/users_test.go +++ b/github/users_test.go @@ -330,7 +330,7 @@ func TestUsersService_ListAll(t *testing.T) { fmt.Fprint(w, `[{"id":2}]`) }) - opt := &UserListOptions{Since: 1, PerPage: Ptr(30)} + opt := &UserListOptions{Since: Ptr(int64(1)), PerPage: Ptr(30)} ctx := t.Context() users, _, err := client.Users.ListAll(ctx, opt) if err != nil { @@ -498,7 +498,7 @@ func TestUserListOptions_Marshal(t *testing.T) { testJSONMarshal(t, &UserListOptions{}, "{}") u := &UserListOptions{ - Since: int64(1900), + Since: Ptr(int64(1900)), PerPage: Ptr(30), } From 9d1f6b971aed65cb8455b2c35b9357e5aa0b6e7b Mon Sep 17 00:00:00 2001 From: Dhananjay Mishra Date: Sun, 8 Feb 2026 14:30:41 +0000 Subject: [PATCH 4/4] feedback --- .golangci.yml | 1 - github/examples_test.go | 2 +- github/users.go | 8 +++----- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 12d08ec69f1..36483d72a03 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -417,7 +417,6 @@ linters: - UpdateRuleParameters.UpdateAllowsFetchAndMerge # TODO: Rules - UploadOptions.Label # TODO: Repositories - UploadOptions.Name # TODO: Repositories - - UserListOptions.Since # TODO: Users exclusions: rules: - linters: diff --git a/github/examples_test.go b/github/examples_test.go index d65e4259b7d..989ba8ef86a 100644 --- a/github/examples_test.go +++ b/github/examples_test.go @@ -105,7 +105,7 @@ func ExampleUsersService_ListAll() { if len(users) == 0 { break } - opts.Since = *users[len(users)-1].ID + opts.Since = users[len(users)-1].ID // Process users... } } diff --git a/github/users.go b/github/users.go index 31fe4f3f9a6..d9591da8ae4 100644 --- a/github/users.go +++ b/github/users.go @@ -208,11 +208,9 @@ func (s *UsersService) GetHovercard(ctx context.Context, user string, opts *Hove // UserListOptions specifies optional parameters to the UsersService.ListAll // method. type UserListOptions struct { - // ID of the last user seen - Since *int64 `url:"since,omitempty"` - - // Note: Pagination is powered exclusively by the Since parameter, - PerPage *int `url:"per_page,omitempty"` + // A user ID. Only return users with an ID greater than this ID. + Since *int64 `url:"since,omitempty"` + PerPage *int `url:"per_page,omitempty"` } // ListAll lists all GitHub users.