RFR: Refactoring in preparation for SKARA-1146

Erik Joelsson erikj at openjdk.java.net
Thu Jan 6 22:00:57 UTC 2022


On Thu, 6 Jan 2022 21:39:00 GMT, Kevin Rushforth <kcr 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.
>
> 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?

I agree. It seems I was lazy and just went with the IDE generated extracted method signature.

> 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?)

You are right, this was part of my initial solution for the follow-on PR and I had forgotten about doing it.

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

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


More information about the skara-dev mailing list