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

[Erik Duveblad]

On Fri, 3 Nov 2023 12:04:25 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - final review
>  - refactor
>  - whitespace
>  - do not throw from checkStatus
>  - Merge branch 'master' into pr-correct-jcheck-conf
>  - Fix .jcheck/conf parsing in multiple places
>  - 2082

Ok, so I spent quite a bit of time going through the places where we parse `.jcheck/conf`. I figured out the following:

- for merge PRs we want to always test each commit in the PR with _both_ the target branch's `.jcheck/conf` and, if any commit in the PR updates `.jcheck/conf`, the `.jcheck/conf` present in the commit. This could be optimized to only test with the `.jcheck/conf` in the commit if the current commit, or any parents of the current commit, updated `.jcheck/conf`. But that seemed like a premature optimization given that the number of merge PR that changes `.jcheck/conf` most likely will be very few.
- for regular pull requests we want to ensure that we are always using the `.jcheck/conf` from the target branch _with_ any additional configuration added (or using an overriding `.jcheck/conf` is that is set up). To enable this I moved this logic into `CheckablePullRequest.parseJCheckConfiguration`.

The remainder of the patch is dealing with fall-out from the above two changes and aligning the code with them.

Thanks,
Erik

PS. The tests failed due to JBS being down

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

PR Comment: https://git.openjdk.org/skara/pull/1578#issuecomment-1792406636