RFR: 1797: Add support for /backport pull request command [v2]

[Zhao Song]

On Wed, 1 Feb 2023 22:06:27 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Zhao Song has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java
>>    
>>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java
>>    
>>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java
>>    
>>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>>  - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java
>>    
>>    Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java line 133:
> 
>> 131:                 reply.println("The target branch `" + targetBranchName + "` does not exist");
>> 132:                 return;
>> 133:             }
> 
> This looks like it has been copied from the other method. Would be better to break it out into methods and reused.

Yes, It's copied from the other method. I tried to extract the code into a method but found it's complex. I will have a try later.

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/BackportCommand.java line 208:
> 
>> 206:             var botComment = allComments.stream()
>> 207:                     .filter(comment -> comment.author().equals(bot.repo().forge().currentUser()))
>> 208:                     .filter(comment -> comment.body().contains("Backport for repo `" + repoName + "` on branch `" + targetBranchName + "` was successfully enabled."))
> 
> Instead of parsing the human readable message, I would recommend encoding a more structured data entry in an html comment. That will be easier to parse and less sensitive to future changes of this code. There should be several similar examples of that throughout the bots.

Sure. Will fix it

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java line 340:
> 
>> 338:                 var text = "Creating backport for repo " + repoName + " on branch " + branchName
>> 339:                         + "\n<!--\n /backport " + repoName + " " + branchName + "\n-->" + "\n"
>> 340:                         + PullRequestCommandWorkItem.VALID_BOT_COMMAND_MARKER;
> 
> In `CheckWorkItem`, when adding `/integrate` based on an earlier `/integrate auto`, we don't hide the command. I think we should be consistent and not hide it here either.

Sure

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

PR: https://git.openjdk.org/skara/pull/1466