Skip to content

Conversation

@alexandear
Copy link
Contributor

BREAKING CHANGE: LicensesService.List now accepts ListLicensesOptions parameter for pagination.


  1. Add missing ListLicensesOptions to LicensesService.List, see the query parameters.
  2. Generate iterator method LicensesService.ListIter.
  3. Add an integration test TestLicenses_ListIter showing that LicensesService.List is called at least twice. We have page: 1, per_page: 1, and number of featured licenses: 3. I think it's a good integration test because featured licenses change rarely.

Updates #3976

@alexandear alexandear force-pushed the fix/licenses-list-iter branch from 9e99372 to 3861b4a Compare February 9, 2026 11:04
@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.52%. Comparing base (5bdc954) to head (3861b4a).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
github/licenses.go 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3981      +/-   ##
==========================================
- Coverage   93.52%   93.52%   -0.01%     
==========================================
  Files         207      207              
  Lines       17569    17590      +21     
==========================================
+ Hits        16432    16451      +19     
- Misses        937      938       +1     
- Partials      200      201       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexandear
Copy link
Contributor Author

It's impossible to cover the line return nil, nil, err.

image

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @alexandear!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @zyfy29 - @Not-Dhananjay-Mishra

@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Feb 9, 2026
Copy link
Contributor

@Not-Dhananjay-Mishra Not-Dhananjay-Mishra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @alexandear.
LGTM

@gmlewis
Copy link
Collaborator

gmlewis commented Feb 9, 2026

Thank you, @Not-Dhananjay-Mishra!
Merging.

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Feb 9, 2026
@gmlewis gmlewis merged commit e5024aa into google:master Feb 9, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants