RFR: SKARA-1333: Sponsor label only removed if PR is actually sponsored
Erik Joelsson
erikj at openjdk.java.net
Tue Feb 8 14:37:06 UTC 2022
On Tue, 8 Feb 2022 12:03:16 GMT, Guoxiong Li <gli at openjdk.org> wrote:
> Hi all,
>
> If an author types `/integrate` command, the `sponsor` label is added. When the author becomes the committer and types the command `/integrate` again, the `sponsor` label doesn't been removed. It is good to remove the label `sponsor`, too, just like the labels `rfr` and `ready`. This patch fixes it and adds a test case.
>
> Thanks for taking the time to review.
>
> Best Regards,
> -- Guoxiong
Looks good, just some language nits in comments.
bots/pr/src/main/java/org/openjdk/skara/bots/pr/SponsorCommand.java line 139:
> 137: }
> 138:
> 139: // This method only has one statement now, but it is kept intentionally to meet the change in the future.
I don't think we need a comment for this. To me it still makes sense to keep this method.
bots/pr/src/test/java/org/openjdk/skara/bots/pr/IntegrateTests.java line 1471:
> 1469: assertEquals(0, notPushed);
> 1470:
> 1471: // Mark the PR author as committer
Suggestion:
// Mark the PR author a committer
bots/pr/src/test/java/org/openjdk/skara/bots/pr/IntegrateTests.java line 1477:
> 1475: var committerBot = PullRequestBot.newBuilder().repo(botUser).censusRepo(committerCensusBuilder.build()).build();
> 1476:
> 1477: // Issue an integrate command with being a Committer
Suggestion:
// Issue an integrate command while being a Committer
-------------
PR: https://git.openjdk.java.net/skara/pull/1282
More information about the skara-dev
mailing list