[Investigation] The design for SKARA-1096 about JEP check

Thu Dec 2 15:21:18 UTC 2021

Hello Guoxiong,

On 2021-12-02 02:54, Guoxiong Li wrote:
> Hi all,
>
> I am going to start working on SKARA-1096 [1] about the JEP check which is
> like the CSR check.
> Firstly, I would like to discuss the concrete design with you and
> receive your suggestions.
>
> I propose the following draft design:
>
> *1. Add issue link types `jep of` and `jep for` in the JBS.*

I don't know if this suggestion is feasible, at least not in the short 
term. While the CSR to Bug relationship is very well defined and clear 
cut, JEPs come in many different shapes and sizes. Also as David says, 
this kind of JBS change needs a wider discussion and audience. If you 
still want to try to pursue such a change to JBS, then perhaps bring it 
up on jdk-dev. If you do, I wouldn't frame it as "this is needed for a 
Skara feature". A change like this to the JEP process needs to stand on 
its own merits, and the ability for Skara and other tools to more easily 
find relationships between implementation and JEP issues in JBS would be 
one of the benefits. Just don't expect changes to happen quickly, if at 
all. I have found the lack of a standard way to associate my 
implementation issues with the JEP annoying and confusing though, so 
perhaps this is possible.

Back to SKARA-1096. The suggested Skara feature here was intended to 
work within the currently available environment. Given that, I don't 
think auto discovery of JEPs is feasible. The command as suggested by 
Jon is '/jep NNNNN'. This would associate the JEP issue with the PR, 
similar to how you add additional issues to a PR today with '/issue 
NNNNN' [1]. The difference would be that a JEP would not be resolved 
when the PR is integrated, it would just serve as another integration 
blocker until the JEP transitions to targeted state. It may also add the 
jep label similar to how the csr label is used today. I'm not completely 
sure we need a label though. We will need to track the relationship to 
the JEP issue anyway somewhere in the PR body or comment, similar to how 
issues are tracked. We also want to augment the body to link to the JEP 
in a way that makes it clear to users. This association isn't just about 
blocking integration but also to make the JEP more visible.

Regarding the new label, there is no extra action needed. The bots can 
add any labels they want without any pre-configuration needed in Github.

The '/jep' command could accept both JBS issue numbers as well as JEP 
numbers to make it more flexible and easier to use.

If in the future, if your suggestion for JBS improvements goes through, 
we can add auto discovery of JEPs.

/Erik

[1] 
https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/issue

> This is similar to the `csr of` and `csr for`.
> Currently, the JEP issue and its implementation issue has at least 3
> relationship:
> (1) The implementation issue is a `relates to` link of the JEP issue. Such
> as JDK-8264130 [2]
> (2) The implementation issue is a `Sub-tasks` link of the JEP issue. Such
> as JDK-8276797 [3]
> (3) The implementation issue is a `is blocked by` link of the JEP issue.
> Such as JDK-8269306 [4]
> They should be unified when we use the skara bot to run the automatic job.
> Only the ongoing JEPs, which excludes the finished JEPs,  need to add the
> `jep of` link because the bot don't check the finished JEPs.
> Only when a developer is working on a JEP should he/she add the link by
> himself/herself. So it is not hard to add this link.
> After this SKARA-1096 completed, we can send an email to `
> jdk-dev at openjdk.java.net` to notify all the JEP developers to add such link.
>
> *2. Add `JEP` label in the corresponding Github repository.*
>
> The `JEP` label means that the JEP is being reviewed and has not targeted.
> If a repository or project has the `JEP` feature, we need to add the `JEP`
> label to its Github code repository.
> Currently, I only know that the JDK mainline need to add the `JEP` label.
> I need your help to list the projects which need the `JEP` label.
>
> *3. Add a bot in the skara to automatically check the JEP.*
>
> When the developer submit a PR in the Github, the bot will check the `jep
> for` link of the main issue of the PR.
>
> If the bot finds a `jep for` link which has not targeted, it would complete
> the following things:
> (1) Add the `JEP` label to the PR.
> (2) Add a checkbox item, such as `Change requires a JEP to be targeted`, to
> the `Progress` part of the PR body. Similar to SKARA-1254 [5]. Because of
> this, we don't need to add the JEP integration blocker.
> (3) Add the JEP issue link to the `Issue` part of the PR body. Similar to
> SKARA-286 [6]
>
> If the bot finds that the `jep for` link has been targeted, it would
> complete the following things:
> (1) Remove the `JEP` label of the PR.
> (2) Check the checkbox item which is in the `Progress` part of the PR body.
> Note: the JEP issue which is in the `Issue` part of the PR body won't be
> removed.
>
> If the bot doesn't find the `jep for` link, it won't do any thing. (no
> `JEP` label, no checkbox item, no JEP issue link)
>
> *4. Not add the command such as `/jep`.*
>
> Currently, we have the `/csr` command. Its value is less when the
> SKARA-1071 [7] is integrated.
> We have discussed about removing the `/csr` command when solving SKARA-1071
> [8].
> But finally, we decided to leave it, because the following reason:
> (1) The PR author might not realize that a CSR is needed. So the command
> `/csr needed` can be used by the Reviewer to indicate that a PR needs to
> have a CSR.
> (2) The `/csr unneeded` command can prevent that a PR author with the role
> of Committer withdraw a CSR that a Reviewer had requested and integrate the
> PR without satisfying that requirement.
>
> Back to the command `/jep`, I think the similar situation has not existed.
> (1) The JEP is firstly created to start the discussion. Then the JEP
> implementation issue and PR is created. So we always know that JEP
> implementation issue needs the JEP issue to be targeted(Approved). So the
> command `/jep needed`, like `/csr needed`, is not really needed.
> (2) The JEP developers are some senior developers who can realize the JEP
> is needed.
> (3) Only the project lead can target(approve) the JEP, so we don't need to
> prevent that the developers integrate the PR without targeting the JEP. So
> the command `jep unneeded`, like `csr unneeded`, is not needed.
>
> The current design is shown above.
> Any idea and suggestion are appreciated.
>
> Best Regards,
> -- Guoxiong
>
> [1] https://bugs.openjdk.java.net/browse/SKARA-1096
> [2] https://bugs.openjdk.java.net/browse/JDK-8264130
> [3] https://bugs.openjdk.java.net/browse/JDK-8276797
> [4] https://bugs.openjdk.java.net/browse/JDK-8269306
> [5] https://bugs.openjdk.java.net/browse/SKARA-1254
> [6] https://bugs.openjdk.java.net/browse/SKARA-286
> [7] https://bugs.openjdk.java.net/browse/SKARA-1071
> [8] https://github.com/openjdk/skara/pull/1245