RFR: 1575: Adjust the pull request APIs in HostedRepository [v2]

Guoxiong Li gli at openjdk.org
Fri Sep 2 18:31:46 UTC 2022


On Fri, 2 Sep 2022 17:09:46 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add parameter about max pages and order.
>
> forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java line 147:
> 
>> 145: 
>> 146:     @Override
>> 147:     public List<PullRequest> pullRequests() {
> 
> I get why you want this method for symmetry, but in reality we could never actually call it in Skara if it would actually return all pull requests for a repo. The potential result set is just too large. The JDK repo has over 10k already. This is why the old `pullRequests(ZonedDateTime)` has the maxPages(1) limit. This method needs that too.

Fixed.

> forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java line 158:
> 
>> 156:     public List<PullRequest> openPullRequests() {
>> 157:         // The default `state` parameter is `open` in GitHub which is not same as `GitLab`.
>> 158:         // Here, the `state` parameter is also added to avoid misunderstanding.
> 
> I don't think we need this comment. Same for lines 181:182.

Removed.

> forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabRepository.java line 135:
> 
>> 133:     @Override
>> 134:     public List<PullRequest> pullRequests() {
>> 135:         return request.get("merge_requests")
> 
> We need a maxPages limit here too. I see it's missing for pullRequestsAfter for Gitlab as well, which is a bug and should also be fixed.

Fixed.

-------------

PR: https://git.openjdk.org/skara/pull/1367


More information about the skara-dev mailing list