RFR: 1714: CSR detection only happens on the title bug of a PR [v2]

Erik Joelsson erikj at openjdk.org
Thu Dec 15 22:19:07 UTC 2022


On Thu, 15 Dec 2022 18:38:30 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 217:
>> 
>>> 215:                 pr.addLabel(CSR_LABEL);
>>> 216:             }
>>> 217:             return;
>> 
>> I'm not sure about this behavior. We loop until we find an existing CSR, then print a reply based on the state of that CSR.
>
> Yes, I think this behavior is right because if we could reach the return statement, it means we already found a non-withdrawn CSR.

But what if there are more? If we find one approved, there may also be another that is withdrawn.

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 272:
>> 
>>> 270:         for (var csrIssue : csrIssues) {
>>> 271:             if (!csrIssue.isClosed()) {
>>> 272:                 ret.put("Change requires a CSR request (" + csrIssue.id() + ") to be approved", false);
>> 
>> I think we can make the issue IDs links here.
>
> I thought about this too. But then I realized the links can be found under the 'Issues' part in PR body. Do you think we still need the links here? If so, I will add the links.

I think it would help.

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

PR: https://git.openjdk.org/skara/pull/1443


More information about the skara-dev mailing list