RFR: 2082: Use correct .jcheck/conf in AdditionalConfiguration.java [v3]

Erik Duveblad ehelin at openjdk.org
Thu Nov 30 20:12:37 UTC 2023


On Fri, 3 Nov 2023 20:35:47 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> Erik Duveblad has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   optimize
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 225:
> 
>> 223:         return confOverride == null ?
>> 224:             JCheck.parseConfiguration(localRepo, hash, additional) :
>> 225:             JCheck.parseConfiguration(confOverride, additional);
> 
> We can check whether additional is empty and if so, we just return original and don't need to parse the configuration again.

Good idea, fixed in most recent change.

> jcheck/src/main/java/org/openjdk/skara/jcheck/JCheck.java line 309:
> 
>> 307:                                 null);
>> 308:         return jcheck.checksForRange();
>> 309:     }
> 
> After introduced this method, the previous `checksFor(ReadOnlyRepository repository, Hash hash) ` is only used in test right now, not sure whether we should remove it.

I think I would like to keep it for the tests since those tests will be exercising slightly different code paths (especially with the most recent version)

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

PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1411208006
PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1411208655


More information about the skara-dev mailing list