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

[Erik Joelsson]

On Fri, 27 Oct 2023 08:32:26 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

bots/pr/src/main/java/org/openjdk/skara/bots/pr/AdditionalConfiguration.java line 34:

> 32: 
> 33: public class AdditionalConfiguration {
> 34:     static List<String> get(Optional<JCheckConfiguration> conf, HostUser botUser, List<Comment> comments, boolean reviewMerge) throws IOException {

Using Optional as parameter isn't recommended, especially since we throw if it's empty. Is it truly optional here? I would think we should just `.orElseThrow()` at the parse call.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1259:

> 1257: 
> 1258:             Hash jcheckConfHash = checkablePullRequest.targetHash();
> 1259:             PullRequestCheckIssueVisitor visitor = checkablePullRequest.createVisitor(jcheckConfHash);

Any particular reason to spell out the types here?

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1274:

> 1272:                         pr.repository().forge().currentUser(), comments, reviewMerge);
> 1273:                 checkablePullRequest.executeChecks(localHash, censusInstance, visitor, additionalConfiguration, jcheckConfHash);
> 1274:                 // Don't need to run the second round if confOverride is set.

Strictly speaking, should we regenerate the AdditionalConfiguration for the second run where we use the merged .jcheck/conf?

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

PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1374572674
PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1374577254
PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1374574088