RFR: 1199: Enforce maintainer approval in JBS before allowing to integrate backports into updates projects [v4]

[Erik Joelsson]

On Wed, 5 Jul 2023 17:49:14 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> As Erik said in the description of this issue, currently, this issue only cares about tracking approval labels in the related bugs.
>> 
>> If a repository is set up with the "approval" configuration, pull requests in that repository will require the maintainer's approval in JBS. Otherwise, the pull request will not be considered ready. 
>> 
>> Erik has also provided a design outlining how to configure the "approval" for a repository.
>> 
>> 
>> The simple case, where the labels are the same for every branch in a repository: 
>> 
>> "approval": { 
>>   "request": "jdk17u-fix-request", 
>>   "approved": "jdk17u-fix-yes", 
>>   "rejected": "jdk17u-fix-no", 
>> } 
>> 
>> To reduce the need for changing multiple strings when copying a configuration for a new repository, there is an optional "prefix" field: 
>> 
>> "approval": { 
>>   "prefix": "jdk17u-fix-", 
>>   "request": "request", 
>>   "approved": "yes", 
>>   "rejected": "no", 
>> } 
>> 
>> When there are multiple branches with different labels, having the prefix set per branch can help reduce the size of the configuration significantly: 
>> 
>> "approval": { 
>>   "request": "-critical-request", 
>>   "approved": "-critical-approved", 
>>   "rejected": "-critical-rejected", 
>>   "branches": [ 
>>     "jdk20\.0\.1": { "prefix": "CPU23_04" } 
>>   ] 
>> }
>
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add ApprovalNeededComment

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

> 1510:         if (existingUnapproved) {
> 1511:             messageBuilder.append("⚠️  @").append(pr.author().username())
> 1512:                     .append(" There are still some issues that have not received maintainer approval.")

I would change the message to something like this: "This change has now been reviewed and requires maintainer approval.", where "approval" is a link to the documentation. The documentation link needs to be a configuration option so we can link to different process documentation for different repos.

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

> 1515:                     .append("[Requesting push approval for fixes](https://openjdk.org/projects/jdk-updates/approval.html)");
> 1516:         } else {
> 1517:             messageBuilder.append("@").append(pr.author().username()).append(" All the issues have already got the maintainer approval!");

When looking at this, I think I'm changing my mind a bit around this. I think we should just post the "approval needed" comment once and not edit it with progress. It's enough to have progress in the pr body.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1537#discussion_r1289179965
PR Review Comment: https://git.openjdk.org/skara/pull/1537#discussion_r1289203822