RFR: Refactoring in preparation for SKARA-1146

Kevin Rushforth kcr at openjdk.java.net
Thu Jan 6 21:45:26 UTC 2022


On Mon, 3 Jan 2022 20:59:56 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> While working on SKARA-1146, I felt that I needed to do some refactoring. To ease the review process, I've separated those changes into a separate PR. Nothing should change functionally from this change. Here is what I've done:
> 
> * Created String constants for the special custom fields we have in JBS.
> * In IssueNotifier, created a method for finding the fixVersion for a branch. This code block is actually copied and sprinkled all over the code base, but for now, I will just unify the copies in the same class.

Looks good. I left a couple questions.

bots/notify/src/main/java/org/openjdk/skara/bots/notify/issue/IssueNotifier.java line 408:

> 406:     }
> 407: 
> 408:     private String getRequestedVersion(Repository localRepository, Commit commit, String tagBranch) {

Minor: would `branch` be a better name? Or is it always a tag name in this method?

bots/notify/src/main/java/org/openjdk/skara/bots/notify/issue/JbsBackport.java line 55:

> 53:         var finalProperties = new HashMap<>(primary.properties());
> 54:         finalProperties.put("issuetype", JSON.of("Backport"));
> 55:         finalProperties.remove(RESOLVED_IN_BUILD);

Minor: did you mean to make this change in this PR? It doesn't seem behavior neutral (might be related to the follow-on PR?)

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

Marked as reviewed by kcr (Reviewer).

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


More information about the skara-dev mailing list