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