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

[Guoxiong Li]

On Thu, 1 Sep 2022 22:26:06 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> The commands should just add labels and a specific type of comment to the (at least for now) head issue of a PR. No other actor should be touching the Issues. We may consider having the command automatically/optionally interact will all issues of a PR. When a command is run, it will already automatically trigger a new CheckWorkItem to update the PR state based on the command action.
>
> The role of a CheckWorkItem is to update the (Skara) state of a PR based on other updates to the PR. With that I mean that it polls for PR updates only, no external ones. With regards to approvals, it should be responsible for toggling the approval label, updating the progress checkbox and post the information message about the approval process. It should most definitely not be changing any labels in any of the Issues.

As you stated, the command should only change the head issue and the CheckWorkItem shouldn't change the issues. My understanding is: the `/approval yes` command only adds the approval label to the head issue and the CheckWorkItem doesn't sync the approval label to other issues.

> We need to check all the issues to make this call. That should only be done once for the CheckRun, and that result needs to be used both for updating the label as well as the progress.

But here, you think we should check all the issues to check the approval label. It seems a contradiction in them. Do I miss something or misunderstand something?

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 197:
> 
>> 195:             var issue = issueOpt.flatMap(value -> issueProject() != null ? issueProject().issue(value.shortId()) : Optional.empty());
>> 196:             if (issue.isPresent()) {
>> 197:                 if (issue.get().labelNames().contains(workItem.approvalLabelName())) {
> 
> We need to check all the issues to make this call. That should only be done once for the CheckRun, and that result needs to be used both for updating the label as well as the progress.

Please see the comment I add below.

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

PR: https://git.openjdk.org/skara/pull/1364