RFR: 1851: Fold CSR bot into PR bot [v3]

[Zhao Song]

On Tue, 28 Mar 2023 18:53:25 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Zhao Song has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - fix a problem
>>  - fix a problem
>>  - renamed some classes
>>  - improve PullRequestBotFactoryTest
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRIssueWorkItem.java line 102:
> 
>> 100:                         .flatMap(r -> r.parsePullRequestUrl(uri.toString()).stream()))
>> 101:                 .filter(Issue::isOpen)
>> 102:                 .filter(pr -> bot.getPRBot(pr.repository().namespace()).enableCsr())
> 
> Are you sure about `.namespace()`?

Oops, of course not.  Sorry for the mistake. I will fix it soon.

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRIssueWorkItem.java line 104:
> 
>> 102:                 // This will mix time stamps from the IssueTracker and the Forge hosting PRs, but it's the
>> 103:                 // best we can do.
>> 104:                 .map(pr -> new CheckWorkItem(bot.getPRBot(pr.repository().name()), pr.id(), errorHandler, csrIssue.updatedAt(), true, true))
> 
> We should probably only schedule this if the PullRequestBot in question is configured to handle CSR.

Good idea!

> bots/pr/src/test/java/org/openjdk/skara/bots/pr/CSRBotTests.java line 65:
> 
>> 63:             TestBotRunner.runPeriodicItems(csrIssueBot);
>> 64: 
>> 65:             var csr = issueProject.createIssue("This is an approved CSR", List.of(), Map.of());
> 
> The title is misleading. The CSR isn't approved until the end of this test.

Will fix it.

> bots/pr/src/test/java/org/openjdk/skara/bots/pr/CSRBotTests.java line 94:
> 
>> 92:             TestBotRunner.runPeriodicItems(csrIssueBot);
>> 93:             // The bot should have removed the CSR label
>> 94:             assertFalse(pr.store().labelNames().contains("csr"));
> 
> At this point, the label is removed, but the PR body still doesn't have the checkbox checked. I think the logic for the checkbox and the label need to be unified and consistent.

I think the checkbox should already be checked at that point.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1151041403
PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1150974996
PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1150953651
PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1150960174