RFR: 2084: Add a `/branch` command similar to `/tag` to create a new branch from a commit [v2]

Erik Duveblad ehelin at openjdk.org
Mon Nov 6 14:27:30 UTC 2023


On Fri, 3 Nov 2023 20:37:46 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Erik Duveblad has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reviewer feedback
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/BranchCommand.java line 110:
> 
>> 108:                 if (ref.isEmpty()) {
>> 109:                     throw new IllegalStateException("Cannot find hash for remote branch '" + branchName + "'");
>> 110:                 }
> 
> I think this turned out a little awkward, especially the exception that should never actually happen.
> Suggestion:
> 
>             var existingBranch = remoteBranches.stream()
>                     .filter(b -> b.name().equals(branchName))
>                     .findAny();
>             if (existingBranch.isPresent()) {

Sure, I rewrote it a bit in the latest version.

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/BranchCommand.java line 127:
> 
>> 125:             log.info("Pushing branch '" + branch + "' to refer to commit: " + commit.hash().hex());
>> 126:             localRepo.push(commit.hash(), bot.repo().authenticatedUrl(), branch.name(), false, false);
>> 127:             log.info(String.join(", ", localRepo.branches().stream().map(Branch::name).collect(Collectors.toList())));
> 
> This will potentially be a rather long log message. Not sure we need to list all branches in the repo.

Yeah, sorry, this was a left-over debug message.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1583#discussion_r1383418653
PR Review Comment: https://git.openjdk.org/skara/pull/1583#discussion_r1383417871


More information about the skara-dev mailing list