[Investigation] Add JEP check to the PR

Guoxiong Li lgxbslgx at gmail.com
Wed Apr 13 19:46:45 UTC 2022


Hi mark,

Thanks for the comment.

> We’ve targeted and integrated over three hundred JEPs since the
> inception of the process in 2011.  How many of those, besides JEP
> 295, were integrated prematurely?

I have not counted the number of such JEPs like JEP-295.
But I know such mistakes have already occured before and similar
mistakes may occur in the future if we don't restrict it.

> The solution you offer requires the JEP implementor to remember to
> type the `/jep` command in the PR.  They could forget to do that
> just as easily as they forget to check the targeting status of the
> JEP itself.

When the JEP PR author integrates the patch without checking the targeting
status,
it happens at a moment and then the author needs to revert and redo the
patch.
If we have the `/jep` command, the author and all the reviewers can use it
and
all the readers (who are not reviewers) of the JEP PR can remind it.
Considering the JEP PR review time may be from 1 week to 1 month,  I think
it
is almost impossible to forget to use the command `/jep`.
After using the command `/jep`, all the following work will be finished by
the bot,
and the author doesn't need to worry about it in the future.

> Now if you could automagically determine that a PR issue is a JEP
> implementation and block it when the JEP is not targeted, without
> the JEP implementor having to do anything special, then that might
> be more compelling

I have proposed a completely automagical design[1] at the beginning,
but it needs to change the JBS and needs more people and more time to
complete the work.
Erik and I have discussed the design[2][3].
In order to firstly meet the request from the developers and let the
developers
use the feature as soon as possible, I proposed this new design.

> I have to ask: Is this really worth the trouble?
> but still, is it a problem worth solving?

It looks like an economics and management problem,
but actually it is a personal emotional problem.
I will analyse it from both aspects.

*1. economics and management aspect*

To implement this feature, we cost the following time:
*cost_time = discussion time + coding time + review time + maintenance time
(if have bug) + others*

We may reduce the following time:
jep_find_time: Everytime we review a JEP PR, we may spend time finding the
original JEP to read.
                       The newly added JEP link can reduce the time.
jep_confirm_time: Everytime we want to integrate a JEP PR, we need to spend
time to confirm the JEP status.
                       If the JEP has not been targeted, we need to confirm
several times.
revert_time: when we make a mistake like JEP-925, we need to revert the
patch.
redo_time: After reverting the patch, we need to redo it.

So all the time we reduce is:
*reduce_time = (jep_find_time + jep_confirm_time + revert_time + redo_time
+ others) *
** (JEP PRs per year or mistake number per year)*
** (the years that JDK Project will live in the future)*

So the problem `is it a problem worth solving?` can change to `does the
reduce_time can be larger than the cost_time?`.
We can know that the answer depends on how many years the JDK Project will
live.

*2. Personal emotional aspect*

Please read the SKARA command reference[4] in the wiki.
You can find there are many commands that we have not used and unnecessary,
especially the `/open` command.
When we firstly read the `/open` command, we may think the Github already
have the `open` button,
we don't need this command totally.
When I meeted the situation that I couldn't re-open the PR which was closed
by the bot[5],
I found the `/open` command and understood that the `/open` command
is very good and I thanked the SKRA developers for providing such a feature.
Back to the `/jep` command, maybe now @mark you don't think it is necessary
to have such a feature.
But when you make the mistake like the developer @vicente in the JEP-925
and spend the extra time
reverting/redoing the patch which actually can be avoided, you may think I
need such a feature like `/jep`.

When @Jonathan reported this feature enhancement [6], I believe he was from
the developers' point of view and
wanted to help the developers to reduce mistakes and not waste unnecessary
time. It is also what I think now.
It is also currently what the SKARA project needs to do.

Best Regards,
-- Guoxiong

[1]
https://mail.openjdk.java.net/pipermail/skara-dev/2021-December/005481.html
[2]
https://mail.openjdk.java.net/pipermail/skara-dev/2021-December/005490.html
[3] https://mail.openjdk.java.net/pipermail/skara-dev/2022-March/005770.html
[4] https://wiki.openjdk.java.net/display/SKARA
[5] https://github.com/openjdk/jdk/pull/1898#issuecomment-813826446
[6] https://bugs.openjdk.java.net/browse/SKARA-1096


More information about the skara-dev mailing list