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

[Erik Joelsson]

On Tue, 28 Mar 2023 19:43:14 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> In this patch, the whole csr bot module is removed. 
>> 
>> Original CSRPullRequestBot is deleted. The functionality of this bot has been moved to `CheckRun`.
>> 
>> Original CSRIssueBot has been moved to pr bot module. PullRequestBotFactory would generate a CSRIssueBot for every configured Issue project.
>
> 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()`?

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.

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.

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.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1151035004
PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1150970336
PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1150921155
PR Review Comment: https://git.openjdk.org/skara/pull/1492#discussion_r1150949374