RFR: 2167: Unify search methods in IssueProject
Erik Duveblad
ehelin at openjdk.org
Tue Jan 30 16:17:12 UTC 2024
On Tue, 30 Jan 2024 14:16:00 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> Hi all,
>>
>> this patch unifies the various ways of searching that exist within `JiraProejct`. I also chose to expose the `search` method in the `IssueProject` since I will make use of it in later patches. This is the first patch of _many_ that will extract the JBS specific parts from `JiraProject` so that `JiraProject` will be about JIRA and JBS specific functionality will be moved elsewhere. This refactoring is needed to allow other parts of the Skara to use JBS functionality with only an `IssueProject` instance (without turning `JiraProject` into effectively `JbsProject`).
>>
>> Thanks,
>> Erik
>
> issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java line 508:
>
>> 506:
>> 507: if (count < issues.get("total").asInt()) {
>> 508: startAt += issues.get("issues").asArray().size();
>
> Aren't `count` and `startAt` basically equivalent at this point? Seems a bit weird to have two values for basically the same thing but calculated differently.
Yes, I just tried to change the existing code as little as possible to try to keep the patch about refactoring. Since I will be touch this a bit more based on your other comment then I can clean this up as well.
> issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java line 513:
>
>> 511: .param("startAt", String.valueOf(startAt));
>> 512: if (limit > 0) {
>> 513: req = req.param("maxResults", Integer.toString(limit));
>
> Is the limit supposed to be the total limit or max size per page (would be great with javadoc specifying this btw)? The `maxResults` parameter is results returned per page. I would think a global limit to be more useful.
Yeah, it was meant to be a global limit (we only have one caller currently passing a limit other than -1, and that caller passes 1, so it would have worked, but I agree that the current code is wrong). Will update!
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1610#discussion_r1471408460
PR Review Comment: https://git.openjdk.org/skara/pull/1610#discussion_r1471407563
More information about the skara-dev
mailing list