[Proposal] Maintainer Approvals and SKARA

[Severin Gehwolf]

Hi,

On Tue, 2022-08-09 at 04:25 +0800, Guoxiong Li wrote:
> Hi all,
> 
> I re-read your comments about this enhancement recently.
> And I also made a mistake when I backport a patch to
> jdk11u-dev [1] although I have backported several times.
> So SKARA must direct the developers accurately and help 
> developers complete the duplicated work to avoid the unnecessary
> mistakes.
> 
> Now I propose the following dev flow. Please only focus on the dev
> flow instead of the code implementation here.
> 
> - When a backport PR is created in Github.
> 1. A new checkbox item `[ ] Change must be properly approved by the
> maintainers` 
> will be added to the `Progress` part of the current PR body.
> 2. A comment is posted by the bot like below (the italic content) to
> direct the developers:
> This is a backport pull request. Please add a comment to the main
> issue [JDK-XXXX](link)
> to state the related condition (the backport reason, the risk of the
> patch, the amount of
> work and so on). Below is an example for you:
> ```
> Fix Request(jdk17u-dev)
> The code applies cleanly and the test in this change fails without
> the patch and succeeds after applying it.
> The risk of this backport is low because the change is little and the
> issue fixed by this change also exists in jdk xy.
> ```
> If you don't have permission to add a comment in JBS. Please use the
> command `request-approval` to provide
> the related content, then the bot can help you add a comment by using
> the content you provided. Below is an example for you:
> ```
> /request-approval
> Fix Request(jdk17u-dev)
> The code applies cleanly and the test in this change fails without
> the patch and succeeds after applying it.
> The risk of this backport is low because the change is little and the
> issue fixed by this change also exists in jdk xy.
> ```
> Please note you have to contact the maintainers directly in the
> issue [JDK-XXXX](link) 
> or by using the command `request-approval` repeatedly instead of in
> this pull request.
> And you don't need to add the fix request label manually to the issue
> like before,
> now the bot can help you add the label automatically when this pull
> request is ready for maintainers to approve.
> 
> - When the backport PR is ready for approval (other checks have
> succeeded and other progresses have been done)
> 1. The bot adds a label named `jdkXXu-fix-request` to all the issues
> of the PR
> 2. The bot adds a blocked label named `approval` in PR, like `csr` or
> other blocked labels
> These two labels are convenient for maintainers to filter the issues
> and PRs which need to be handled now.
> 
> - It is time for maintainers to approve
> The maintainers can add the label `jdkXXu-fix-yes` or `jdkXXu-fix-no`
> in the issue or use the command 
> `/approval yes|no|y|n` in the PR to approve or object the patch. This
> newly added command `approval` 
> can be very convenient if one backport PR has many corresponding
> issues. After using this command,
> the bot will add the related label to all the issues just like the
> label `jdkXXu-fix-request` it had added.
> 
> - Then the commit or sponsor flow will continue as usual.
> After approval, if the maintainer said 'yes', the PR will become
> ready to be integrated.
> If the maintainer said 'no', the PR can be closed by the bot with a
> comment like
> "The maintainers disapproved of this patch so this pull request will
> be closed automatically".
> No matter 'yes' or 'no', the label `approval` in the PR will be
> removed.
> 
> Above is all the dev flow. And next, I will give more information
> about the new commands `request-approval` and `approval`.
> 
> - "request-approval" command
> It is similar to the `summary` command which permits multiline
> content. And this command can be used 
> multi times if the author of the PR wants to. Each time this command
> is used, the bot will post the content 
> to a new comment in the issue. Because the author who has no
> permission may want to contact/talk with the maintainers.
> The related record will be recorded in the issue instead of PR which
> is your intention in your previous comments.
> 
> - "approval" command
> The command `/approval yes` or `/approval y` mean `approved`.
> And the command `/approval no` or `/approval n` mean `dispproved`.
> This command can be used multiple times and only the last time is
> valid.
> Please note when the `/approval no` or `/approval n` is used, the bot
> will close the PR,
> and when the `/approval yes` or `/approval y` later is used, the bot
> will open the PR automatically.
> 
> 
> That is all. Thanks for reading and providing feedback. 
> And I will try to implement these features if I have time and hear no
> objection after discussion.
> 

This sounds good to me. +1. Perhaps we should move this discussion and
implementation proposal to the bug tracking this issue as it is hard to
find those mailing list posts later on?
https://bugs.openjdk.org/browse/SKARA-1199

It also seems to be in-line with what Andrew Hughes suggested in [i].

Thanks,
Severin

[i] https://mail.openjdk.org/pipermail/jdk-updates-dev/2022-March/013278.html

> [1] 
> https://github.com/openjdk/jdk11u-dev/pull/1218#issuecomment-11842356
> 01
> 
> Best Regards,
> -- Guoxiong