RFR: 1850: CSR bot can pick up wrong fix version if PR branch is behind target branch [v2]

Zhao Song zsong at openjdk.org
Wed Apr 5 20:29:31 UTC 2023

On Wed, 5 Apr 2023 19:32:46 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Zhao Song has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>>   SKARA-1850
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 106:
>> 104:             var csrLinks = new StringBuilder();
>> 105:             for (Matcher csr : csrs) {
>> 106:                 csrLinks.append("[").append(csr.group(1)).append("](").append(csr.group(2)).append(")").append(" ");
> I think this can be moved into the conditional block below.

Yes, it will be better

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 116:
>> 114:                     .flatMap(comment -> comment.body().lines())
>> 115:                     .filter(line -> line.startsWith("/csr"))
>> 116:                     .collect(Collectors.joining());
> I think we only need to look for the last `CSR_NEEDED_MARKER` or `CSR_UNNEEDED_MARKER`. Unless the csr needed state changes from the /csr command, is there anything for the `CheckRun` to update? Reacting to the user issuing `/csr` seems wrong to me because we need the `PullRequestCommandWorkItem` to have finished processing it before `CheckRun` can find anything to act on.

Think more about it and now I think we don't need to look for anything here. I will remove this code.


PR Review Comment: https://git.openjdk.org/skara/pull/1496#discussion_r1158977105
PR Review Comment: https://git.openjdk.org/skara/pull/1496#discussion_r1158975786

More information about the skara-dev mailing list