[Proposal] Maintainer Approvals and SKARA

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Mon Aug 15 06:11:04 UTC 2022


Hi,

+1, This is a good improval.

Please remember that the instructions should not talk about backports, 
but about changes to an updates repository. 
Sometimes there are genuine changes for an update repository, 
these have to follow the very same approval rules.

> > > 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:
"This is a change to an updates repository. Please add a comment to 
the JBS issue, in case of a backport to the existing main issue [JDK-XXXX](link).
State the related condition (Why you do the change/backport, 
the risk of the patch, the amount of work and so on). Below is
an example for you:   ...

Best regards, Goetz


> -----Original Message-----
> From: skara-dev <skara-dev-retn at openjdk.org> On Behalf Of Langer, Christoph
> Sent: Montag, 15. August 2022 08:02
> To: Severin Gehwolf <sgehwolf at redhat.com>; 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 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



More information about the jdk-updates-dev mailing list