[Proposal] Maintainer Approvals and SKARA

Thu Mar 24 16:46:21 UTC 2022

Hi Erik, all,

I completely second both points:

1. The fact that the maintainer/approval process could be improved from a tooling perspective and
2. In the end, the information has to be persisted in JBS.

I very much like your analysis, Erik, and I think I would welcome a hybrid solution.
So there should still be the labels and request like we have now. A missing approval label should be an integration blocker in the PR. And if we'd in the end get some commands, like /approve which only maintainers are allowed to issue and /request-approval <comment> which adds the request to JBS, also enabling non JBS users to request approval, it would greatly help.

Thanks & Best regards
Christoph

> -----Original Message-----
> From: jdk8u-dev <jdk8u-dev-retn at openjdk.java.net> On Behalf Of
> erik.joelsson at oracle.com
> Sent: Donnerstag, 24. März 2022 15:02
> To: Andrew Hughes <gnu.andrew at redhat.com>; jdk8u-dev at openjdk.java.net;
> jdk-updates-dev at openjdk.java.net
> Subject: Re: [Proposal] Maintainer Approvals and SKARA
> 
> Hello,
> 
> I agree that solving SKARA-1199 would provide quite a bit of benefit to the
> update releases. The current process works, but it requires knowledge and
> manual care to not make mistakes. One of the main ideas with Skara was to
> codify our processes to make them more foolproof and easy to follow. I
> definitely think this approval process fits the bill as something we should
> support better with tooling.
> 
> Breaking apart the problem a bit, from the perspective of a user developing a
> fix for an update release that requires approval, I agree with Kevin that the
> current state of the approval should be reflected as a "progress" on the PR.
> That makes it very clear if the PR is ready for integration or not and fits in with
> how Skara tracks and treats other integration blockers. Basing this functionality
> around the model of the /sponsor command would be a pretty major shift in
> my opinion. It moves the ability to actually integrate, and as such the control of
> the timing, from the contributor to the integrator. This mimics how some other
> big open source projects handle things (e.g. the Linux kernel). I think it makes
> sense to do this when sponsoring, because we are then helping new and
> inexperienced (in OpenJDK processes at least) users, which helps prevent
> mistakes. For contributors of committer status or above, I think we should
> retain their ability to control the timing of integration. Implementation wise, I
> don't think it makes a big difference.
> 
> The other part of the problem, which I think is what this discussion should
> really focus on, is where the official approval takes place. We can keep it in JBS
> like today, or we can move it to the PR, as suggested by Andrew. From an
> implementation point of view, moving it to the PR is less work, but I don't think
> we should base this decision on process on if it takes an extra day to
> implement. The tooling should support the process we want.
> 
> Without having spent a lot of time thinking about it so please tell me if I'm
> missing something, would it be possible to support a hybrid/dual solution? The
> official process could still be the labels in the JBS issues, but the Skara
> 'integrator'-only /approve command on a PR could also add the appropriate *-
> fix-yes label to all associated bugs. There would also be a Skara bot monitoring
> JBS and automatically updating the approval status on PRs based on labels in
> JBS. This would shortcut the process for integrators that prefer working
> through PRs, while integrators that prefer to work through JBS can continue
> the same way as before. I think this should alleviate the issue of backporting
> many bugs with one PR. This is of course also the biggest change to implement,
> but not that much more than what I had already outlined in SKARA-1199.
> 
> /Erik
> 
> 
> On 2022-03-23 13:13, Andrew Hughes wrote:
> > 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/845d40ae217ff47ec53e2fd655b95
> 9
> > 33aa35eac6 [1] https://bugs.openjdk.java.net/browse/SKARA-1384
> >
> > Thanks,