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

Erik Duveblad ehelin at openjdk.org
Fri Nov 3 13:10:53 UTC 2023


On Fri, 27 Oct 2023 13:25:30 GMT, Erik Joelsson <erikj 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/AdditionalConfiguration.java line 34:
> 
>> 32: 
>> 33: public class AdditionalConfiguration {
>> 34:     static List<String> get(Optional<JCheckConfiguration> conf, HostUser botUser, List<Comment> comments, boolean reviewMerge) throws IOException {
> 
> Using Optional as parameter isn't recommended, especially since we throw if it's empty. Is it truly optional here? I would think we should just `.orElseThrow()` at the parse call.

Thanks, removed the use of `Optional` in the latest version.

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1259:
> 
>> 1257: 
>> 1258:             Hash jcheckConfHash = checkablePullRequest.targetHash();
>> 1259:             PullRequestCheckIssueVisitor visitor = checkablePullRequest.createVisitor(jcheckConfHash);
> 
> Any particular reason to spell out the types here?

Nope, thanks for catching

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

PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1381656560
PR Review Comment: https://git.openjdk.org/skara/pull/1578#discussion_r1381655725


More information about the skara-dev mailing list