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