[Proposal] Maintainer Approvals and SKARA

Thu Mar 24 14:01:53 UTC 2022

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/845d40ae217ff47ec53e2fd655b95933aa35eac6
> [1] https://bugs.openjdk.java.net/browse/SKARA-1384
>
> Thanks,