RFR: 2082: Use correct .jcheck/conf in AdditionalConfiguration.java [v3]

[Zhao Song]

On Fri, 3 Nov 2023 13:10:52 GMT, Erik Duveblad <ehelin at openjdk.org> wrote:

>> Hi all,
>> 
>> please review this patch that ensures that we pass the correct `.jcheck/conf` to [AdditionalConfiguration.java](https://github.com/openjdk/skara/blob/master/bots/pr/src/main/java/org/openjdk/skara/bots/pr/AdditionalConfiguration.java). The `.jcheck/conf` should either come from the "overriding" `.jcheck/conf` (if the PR bot is configured to use that) or the pull request's target branch.
>> 
>> I have added that test that passes with this patch but fails without it.
>> 
>> Thanks,
>> Erik
>
> Erik Duveblad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   optimize

This patch looks pretty good, just some minor issues to address.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 225:

> 223:         return confOverride == null ?
> 224:             JCheck.parseConfiguration(localRepo, hash, additional) :
> 225:             JCheck.parseConfiguration(confOverride, additional);

We can check whether additional is empty and if so, we just return original and don't need to parse the configuration again.

jcheck/src/main/java/org/openjdk/skara/jcheck/JCheck.java line 309:

> 307:                                 null);
> 308:         return jcheck.checksForRange();
> 309:     }

After introduced this method, the previous `checksFor(ReadOnlyRepository repository, Hash hash) ` is only used in test right now, not sure whether we should remove it.

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

PR Review: https://git.openjdk.org/skara/pull/1578#pullrequestreview-1713494322
PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1382175193
PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1382197725