RFR: 2045: Make maintainer approval feature compatible with dependent pr feature.

[Erik Joelsson]

On Wed, 27 Sep 2023 19:50:25 GMT, Zhao Song <zsong at openjdk.org> wrote:

> A user reported that a dependent pr in a repo which configured with maintainer approval feature is not marked for approval. 
> 
> After investigation, I realized that the skara bot would determine whether this pr needs maintainer approval by checking whether merging into the target branch needs maintainer approval. 
>   
> In this case, we only configured that merging into master branch of jdk21u needs maintainer approval, however, for dependent pull requests, the target branch is pr/XXX. 
> 
> To fix this issue, we should let the skara bot be able to find the real target ref.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/ApprovalCommand.java line 185:

> 183:         var dependentPR = pr.repository().pullRequest(id);
> 184:         return realTargetRef(dependentPR);
> 185:     }

There is a class `org.openjdk.skara.forge.PreIntegrations` that handles these things. I think this method should be added there (and be integrated with existing methods).

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

> 106:                 workItem.bot.confOverrideRef(),
> 107:                 comments);
> 108:         realTargetRef = ApprovalCommand.realTargetRef(pr);

I would like to avoid calculating this for every pr targeting a pr/X branch. We could make this field lazy initialized in the `needsApproval` method. Something like:


if (approval != null) {
    if (realTargetRef == null) {
         realTargetRef = PreIntegration.realTargetRef(pr);
    }
    return approval.needsApproval(realTargetRef);
}
return false;


I would also add a comment on the field with something like

// Only set if approval is configured for the repo

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

PR Review Comment: https://git.openjdk.org/skara/pull/1562#discussion_r1339232687
PR Review Comment: https://git.openjdk.org/skara/pull/1562#discussion_r1339244487