[Proposal] Maintainer Approvals and SKARA

Thu Mar 24 14:22:22 UTC 2022

Hi,

Basically I would appreciate some more guidance by Sakra for the 
backports. 

Blocking /integrate until the change is approved makes sense.
Restricting approval to maintainers makes sense, too.

I would not force sequentialization of review and approval though.
The maintainer can decide by himself whether the review
is needed to approve the change. If both, review and approval, 
are enforced by Skara before a change can be integrated, 
the maintainer should be able to approve a change
before it is reviewed if he thinks the change is definitely needed.
This would make backporting and the maintainer
task much more efficient.  (Maybe the maintainer
can have a command /requires_review_for_approval).

Also, I would not want to move all work, or, more precisely, all the 
documentation, to the PR.
We do not need the tags in JBS if approval is handled on the 
PR. But what I want to see in JBS is the comment that states
 * why the change should be backported
 * the risk of the backport
 * the amount of work done on top of the original change
Pull requests contain all sort of data, and often there are 
several pull request until one of them gets pushed. 
Is it possible to have a command /request_approval
that adds the rest of the comment as a comment to JBS?
So that I can write in the PR:

The issue fixed by this change also exists in jdk xy.
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 
very local. 
/request_approval

And all of the comment is added to JBS under a heading "Fix request xy".

Similar, when the maintainer says
/approval-yes or -no, a comment should be added to the 
JBS issue.

The JBS is the basic source of information for me
when I want to approve a change. 
It lists related issues  and the discussion in the 
comments often is helpful. Therefore the comment 
should end up there.

And one thing that is important: there must be a way
to filter for the PRs that need approval ��

Just my 5ct ��

Best regards,
  Goetz.

> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> On
> Behalf Of Andrew Hughes
> Sent: Wednesday, March 23, 2022 9:14 PM
> To: jdk8u-dev at openjdk.java.net; jdk-updates-dev at openjdk.java.net
> Subject: [Proposal] Maintainer Approvals and SKARA
> 
> Hi all,
> 
> We have been discussing how to implement and/or adapt the current
> approval process (jdk<x>-fix-request, jdk<x>-fix-yes, etc.) to SKARA
> on this bug: https://bugs.openjdk.java.net/browse/SKARA-1199
> 
> At present, we have been continuing to approve bugs for update
> releases as we did in Mercurial; when the change is reviewed, the
> proposer should flag it as jdk<>-fix-request (or
> jdk<>-critical-request for a rampdown fix) and then an approver
> responds with the *-yes or *-no response on the bug.
> 
> SKARA has introduced a new worry for me into this process, due to the
> way the PR bot directs the user through the change process. This in
> itself is a good thing. However, while it is currently not aware of
> the approval process, it can encourage someone to integrate a change
> which has not yet been approved. Hence why I feel some solution to
> SKARA-1199 is quite critical (especially with the potential move of
> the master 8u repository to SKARA) and it surprises me that other
> update projects have not pushed harder for this, prior to 8u adopting
> SKARA.
> 
> In thinking through how to implement this in SKARA, it occurred to me
> that trying to continue with what we have now may not be the best
> solution. The issues I see are:
> 
> 1. Having the approval process in the bug database separate from the
> proposed commit on GitHub creates a disconnect between the two.
> We had this before, with the separation between mailing list activity
> and the bug database, but now that pretty much everything else happens
> in GitHub/SKARA, it feels odd to have to go back and forth between
> JBS and GitHub to approve a change.
> 
> 2. Access to add a label in JBS requires someone to have JDK authorship
> status, so, for non-authors, someone else has to do this on their
> behalf (as with sponsoring)
> 
> 3. On the flip-side to #2, any JDK author can add jdk<x>-fix-yes or
> jdk<x>-critical-yes to a bug, as I believe has happened in the past.
> 
> 4. From a technical perspective, this means JBS has to be regularly
> probed to pick up new labels. This also affects CSRs, which already
> have issues in this area, and the demand for backports would be
> much higher.
> 
> 5. When we've dealt with backports which affect multiple bugs,
> the informal process we've had in the past means we've been able
> to choose whether to require that all bugs are flagged or not.
> Having a formal version of the process we have now in SKARA would
> mean having to label every referenced bug in something like [0].
> 
> My proposal would be that we shift approvals to SKARA in a similar
> way to which sponsorship is currently handled:
> 
> 1. When a PR for a backport project reaches the point at which
> it would currently be integrated (after either /integrate for
> a committer or /sponsor for a non-committer), it instead
> gets an 'approval' label.
> 
> 2. For the PR to move forward, an 'integrator' (someone capable
> of merges & tags) [1] would need to perform '/approval [yes|no]' to
> cause the change to either be integrated or rejected.
> 
> I believe this would simplify the implementation of this feature
> a lot and also integrate it better into SKARA.
> 
> The bot could still label bugs with the jdk<x>-fix-request when
> it adds the 'approval' label to the PR and with jdk<x>-fix-yes
> and jdk<x>-fix-no when the /approval command is used.
> 
> The use of jdk<x>-fix-request or jdk<x>-critical-request can
> also be pre-determined by the bot, based on the repository
> the PR is against (jdk<x>u-dev for the former, jdk<x>u for the
> latter).  This is something that would go in the server-side
> bot configuration.
> 
> Thoughts?
> 
> I'm honestly struggling to see why we'd hold onto the existing
> process, but I also realise that adaptation can be hard, particularly
> for other update projects where SKARA has been in use longer.
> 
> [0]
> https://github.com/openjdk/jdk8u/commit/845d40ae217ff47ec53e2fd655b9
> 5933aa35eac6
> [1] https://bugs.openjdk.java.net/browse/SKARA-1384
> 
> Thanks,
> --
> Andrew :)
> Pronouns: he / him or they / them
> Senior Free Java Software Engineer
> OpenJDK Package Owner
> Red Hat, Inc.
> (https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww
> .redhat.com%2F&data=04%7C01%7Cgoetz.lindenmaier%40sap.com%7C
> 2cf36bbd0cb84c6d579b08da0d09d818%7C42f7676cf455423c82f6dc2d99791af7
> %7C0%7C0%7C637836633177510023%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000&sdata=v%2Fv1UexHL%2Fz%2BX3kSQTcClsNWH2BRebaOWqPEP0gE
> VTE%3D&reserved=0)
> 
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222