RFR: SKARA-758: Add /check pull request command [v3]

Robin Westberg rwestberg at openjdk.java.net
Mon Nov 23 10:39:32 UTC 2020

On Fri, 20 Nov 2020 12:30:47 GMT, Kartik Ohri <github.com+27751938+amCap1712 at openjdk.org> wrote:

>> Hi!
>> Kindly review the patch to allow manual execution of jcheck. I think it might be worthwhile to restrict this command to some specific people but I am not sure who that should be (maybe restricting to the author is enough but a reviewer might want to execute it as well in case its urgent). 
>> I wanted to test this locally by setting up the skara bots on my own repository but was unable to figure out how to do so due to lack of documentation.
>> Thanks.
>> Regards,
>> Kartik
> Kartik Ohri has updated the pull request incrementally with one additional commit since the last revision:
>   Add test for /check command

Changes requested by rwestberg (Reviewer).

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckCommand.java line 24:

> 22:     public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply) {
> 23:         try {
> 24:             var workItem = new CheckWorkItem(bot, pr, e -> {});

Don't think you actually have to do the check here, `CommandWorkItem` will schedule an additional check after every command anyway. But to force the check to run you'll need to ensure that the "metadata" of the existing check becomes invalid. If you update `metadataComments` in `CheckWorkItem` to add a special reply marker from the `/check` command, and ensure that the check command outputs it, I think thinks should work.

bots/pr/src/test/java/org/openjdk/skara/bots/pr/CheckCommandTests.java line 39:

> 37:             localRepo.push(editHash, author.url(), "edit", true);
> 38:             var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");
> 39:             pr.addComment("/check");

Looks like a good start, but it's possible that this test is a bit too simple. Would it still pass if you removed this line (as checking will occur anyway)?


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

More information about the skara-dev mailing list