-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat!: Add pagination to Repository/Organization Rules Get methods #3236
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3236 +/- ##
==========================================
- Coverage 97.72% 92.89% -4.83%
==========================================
Files 153 171 +18
Lines 13390 11642 -1748
==========================================
- Hits 13085 10815 -2270
- Misses 215 730 +515
- Partials 90 97 +7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @lyoung-confluent !
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I responded before the Codecov report.
Please address these issues first.
@@ -38,7 +39,7 @@ func TestOrganizationsService_GetAllOrganizationRulesets(t *testing.T) { | |||
}) | |||
|
|||
ctx := context.Background() | |||
rulesets, _, err := client.Organizations.GetAllOrganizationRulesets(ctx, "o") | |||
rulesets, _, err := client.Organizations.GetAllOrganizationRulesets(ctx, "o", &ListOptions{Page: 2, PerPage: 2}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After line 63 below, please add:
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Organizations.GetAllOrganizationRulesets(ctx, "\n", &ListOptions{})
return err
})
@@ -301,7 +302,7 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { | |||
}) | |||
|
|||
ctx := context.Background() | |||
rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch") | |||
rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", &ListOptions{Page: 2, PerPage: 2}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After line 329 below, please add:
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Repositories.GetRulesForBranch(ctx, "\n", "\n", "\n", &ListOptions{})
return err
})
@@ -350,7 +352,7 @@ func TestRepositoriesService_GetRulesForBranchEmptyUpdateRule(t *testing.T) { | |||
}) | |||
|
|||
ctx := context.Background() | |||
rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch") | |||
rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", &ListOptions{Page: 2, PerPage: 2}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After line 369 below, please add:
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Repositories.GetRulesForBranch(ctx, "\n", "\n", "\n", &ListOptions{})
return err
})
@@ -400,7 +403,7 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { | |||
}) | |||
|
|||
ctx := context.Background() | |||
ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", false) | |||
ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", false, &ListOptions{Page: 2, PerPage: 2}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After line 431 below, please add:
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Repositories.GetRulesets(ctx, "\n", "\n", false, &ListOptions{})
return err
})
This PR will be closed and marked as abandoned if there is no reply by next week. |
BREAKING CHANGE:
OrganizationsService.GetAllOrganizationRulesets
,RepositoriesService.GetRulesForBranch
, andRepositoriesService.GetAllRulesets
now haveopts *ListOptions
.The Organization and Repository Rules APIs support pagination (
page
andper_page
):This PR adds the standard
ListOptions
parameter to each method so pagination can be used when listing rules.