search

andygrunwald / go-gerrit

Go client/library for Gerrit Code Review
https://godoc.org/github.com/andygrunwald/go-gerrit
MIT License
96 stars 40 forks source link

fix suggertaccout 500 limit #92

Closed Seizens closed 3 years ago

Seizens commented 3 years ago

fix suggertaccount 500 limit AccountInfo struct add MoreAccount parameter to see if the request needs to continue QueryBody Add Start parameter Query start position

image

andygrunwald commented 3 years ago

Hey @Seizens, thanks a lot for the PR. At the first glance, it seems reasonable. However, I want to check if this is somehow related to https://github.com/andygrunwald/go-gerrit/pull/48 and if the S parameter is a general issue or can be solved in a general way, rather than for a single endpoint.

I have limited time right now to dig into it. Please bear with me when this takes a bit of time. Thanks again for your efforts.

andygrunwald commented 3 years ago

Hey @Seizens,

thanks for raising the PR. I merged part of your code, see https://github.com/andygrunwald/go-gerrit/commit/101051e002059e0c8c3143008b47108daf483a60.

I introduced the s directly in QueryOptions, because the parameter is also supported in other cases, see list-changes for example: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes

I hope this will work for you.

andygrunwald commented 3 years ago

Maybe I made an implementation mistake with https://github.com/andygrunwald/go-gerrit/commit/101051e002059e0c8c3143008b47108daf483a60 because of https://github.com/andygrunwald/go-gerrit/pull/48

I will fix this in the next days and add proper testing.

Seizens commented 3 years ago

hello @andygrunwald， thanks you !!! I am honored that my code is useful to you . I find group.go groupInfo will also have number limits， you can see https://gerrit-review.googlesource.com/Documentation/rest-api-groups.html#query-groups here ， If the number of groups matching the query exceeds either the internal limit or a supplied limit query parameter, the last group object has a _more_groups: true JSON field set.

If I have time, I will also make a new PR, and i think the numbers limits will need other place .

the last , i am sorry, my english is just soso.

andygrunwald commented 3 years ago

Hey @Seizens, thanks again for your involvement.

For SuggestAccount, I have implemented your change in https://github.com/andygrunwald/go-gerrit/commit/67c934529a81c2e26f71455b75baaa77f4896e0c

For GroupInfo, I have implemented the _more_groups field in https://github.com/andygrunwald/go-gerrit/commit/ff14d0674afb9b172cf324af84466666c3ac55c9

If you encounter other missing features, feel free to open a new issue or a Pull Request. Thanks again.