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