RFR: 2045: Make maintainer approval feature compatible with dependent pr feature.
Zhao Song
zsong at openjdk.org
Wed Sep 27 21:50:53 UTC 2023
On Wed, 27 Sep 2023 21:33:33 GMT, Erik Joelsson <erikj 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/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
Thanks for the suggestion. It will make this patch much better!
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1562#discussion_r1339258901
More information about the skara-dev
mailing list