RFR: 1554: Fold JEPBot into PR bot [v2]

[Zhao Song]

On Mon, 26 Jun 2023 15:01:13 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> This patch removes the JEPBot and moves the functionality into the pr bot module, similar to what SKARA-1851 did for CSRBot. Instead of introducing a new JEPBot in the pr bot module, I'm using the existing `IssueBot` from SKARA-1912 to poll for updated JEPs to process PRs for. 
>> 
>> The handling of different kinds of "Issues" in CheckRun was (and still to some extent is) rather messy. I've tried to tidy it up a bit in this patch, but didn't want to take it too far in a single change. In CheckRun, all issues are fetched in one place and stored in maps which are then used throughout the run. This also limits the amount of places where failure handling is needed. Hopefully  SKARA-1963 can improve this aspect further. I also didn't want to change too much of the CSR issue handling in this patch, but I see some room for improvement there too.
>> 
>> Now that all JEP issue handling is done in the pr bot module, we don't need to communicate between bots using labels. Despite this, I chose to leave the `jep` label working the same way as before: Adding it when the jep command is issued for adding a jep requirement and removing it when that requirement has been fulfilled, and also re-adding it if the requirement for some reason is no longer fulfilled. I think this will be the least confusing to users. However, one could also imagine the `jep` label just staying around signaling that this was a JEP PR, even after a PR has been integrated.
>> 
>> The JEP test in `CheckTests` has been enhanced to verify that updates to the JEP issue are detected and handled, and that the label is updated accordingly by `CheckRun`.
>> 
>> The issue metadata hash has been extended to include issue state and resolution, so that CheckRun will re-evaluate if any of those fields change for a JEP issue. Since we now have a separate `IssueTrackerIssue` interface, I chose to expose these fields directly through two new accessor methods instead of having to deal with complex and untyped properties everywhere. I've also added default values for status, priority and type for `TestIssueTrackerIssue` so that it mimics `JiraIssue` better in regards to those fields, and removed the null checks in the metadata hash calculation.
>
> Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove jep module

Looks good. Just two trivial issues.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 330:

> 328: 
> 329:         var issueProject = issueProject();
> 330:         if (issueProject != null) {

Do we still need to check `issueProject ` here?

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1399:

> 1397: 
> 1398:             var resolution = csr.properties().get("resolution");
> 1399:             if (resolution == null || resolution.isNull()) {

We could also use `csr.resolution()` here. But the logic here is a little bit complex. Maybe I could do it in a later patch.

test/src/main/java/org/openjdk/skara/test/TestHost.java line 30:

> 28: import org.openjdk.skara.issuetracker.*;
> 29: import org.openjdk.skara.json.JSON;
> 30: import org.openjdk.skara.json.JSONObject;

These imports are never used.

-------------

Marked as reviewed by zsong (Reviewer).

PR Review: https://git.openjdk.org/skara/pull/1535#pullrequestreview-1499580265
PR Review Comment: https://git.openjdk.org/skara/pull/1535#discussion_r1242876538
PR Review Comment: https://git.openjdk.org/skara/pull/1535#discussion_r1242874652
PR Review Comment: https://git.openjdk.org/skara/pull/1535#discussion_r1242907801