RFR: 1385: Backport MR links to the wrong CSR request [v5]

[Erik Joelsson]

On Thu, 12 May 2022 04:14:05 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Hi all,
>> 
>> This patch solves the wrong CSR in the backport PR. It mainly has the following rules:
>> 1. if the `version` in `.jcheck/conf` doesn't exist or the `version` is wrong, the bot think the CSR also doesn't exist.
>> 2. else, if the `version` in `.jcheck/conf` matches the fix version of the primary CSR, the bot will use the primary CSR. (primary CSR is the CSR of the primary issue)
>> 3. else, if these is a backport issue matches the `version`, the bot will find the CSR of the backport issue. If found, use it. If not, the bot think the CSR doesn't exist.
>> 4. else (no backport issue matches the `version`) the bot think the CSR doesn't exist.
>> 
>> The classes `CSRBot`, `CheckRun` and `CSRCommand` will handle the situations above respectively.
>> 
>> Thanks for taking the time to review.
>> 
>> Best Regards,
>> -- Guoxiong
>
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move the methods from IssueUtil to Backports.

Sorry it's taken a while, I've been out sick.

This is starting to look really good, thanks for hanging in there. Only a few language nits.

bots/csr/src/main/java/org/openjdk/skara/bots/csr/CSRBot.java line 93:

> 91:             var versionOpt = getVersion(pr);
> 92:             if (versionOpt.isEmpty()) {
> 93:                 log.info("No right fix version found in file `.jcheck/conf` for " + describe(pr));

Suggestion:

                log.info("No fix version found in `.jcheck/conf` for " + describe(pr));

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 55:

> 53:     private static void linkReply(PullRequest pr, Issue issue, PrintWriter writer) {
> 54:         writer.println("@" + pr.author().username() + " please create a [CSR](https://wiki.openjdk.java.net/display/csr/Main) request for issue " +
> 55:                 "[" + issue.id() + "](" + issue.webUrl() + ") with the right fix version. This pull request cannot be integrated until the CSR request is approved.");

Suggestion:

                "[" + issue.id() + "](" + issue.webUrl() + ") with the correct fix version. " +
                "This pull request cannot be integrated until the CSR request is approved.");

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 86:

> 84:             if (!labels.contains(CSR_LABEL)) {
> 85:                 // FIXME here, the PR may have an approved CSR. We should distinguishing the situations
> 86:                 // about having no csr request and (having csr request && the CSR has been approved).

Suggestion:

                // FIXME here, the PR may have an approved CSR. We should distinguish the situations
                // of having no csr request and having an approved csr request.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 102:

> 100:             if (jbsIssueOpt.isEmpty()) {
> 101:                 pr.removeLabel(CSR_LABEL);
> 102:                 reply.println("determined that a [CSR](https://wiki.openjdk.java.net/display/csr/Main) request " +

This reply is repeated a lot. Perhaps we should have a method for it like some of the other replies have.

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

PR: https://git.openjdk.java.net/skara/pull/1318