[Proposal] Maintainer Approvals and SKARA

[Langer, Christoph]

Hi Guoxiong,

+1 from me as well. Would be absolutely great if this part of the backport process could be improved soon. 😊

Best regards
Christoph

> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.org> On Behalf Of
> Severin Gehwolf
> Sent: Dienstag, 9. August 2022 10:43
> To: Guoxiong Li <lgxbslgx at gmail.com>; jdk8u-dev at openjdk.org; jdk-updates-
> dev at openjdk.org; skara-dev at openjdk.org
> Subject: Re: [Proposal] Maintainer Approvals and SKARA
> 
> 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