RFR: webrev: add logging
Jorn Vernee
jvernee at openjdk.java.net
Thu Jun 18 12:40:06 UTC 2020
On Thu, 18 Jun 2020 12:04:58 GMT, Erik Helin <ehelin at openjdk.org> wrote:
> Hi all,
>
> please review this small patch that add some additional logging to `Webrev` and dereferences `Optional`s a bit more
> carefully in `ModifiedFileView`.
> Testing:
> - [x] `make test` passes on Linux x64
>
> Thanks,
> Erik
Marked as reviewed by jvernee (Reviewer).
webrev/src/main/java/org/openjdk/skara/webrev/ModifiedFileView.java line 56:
> 55: patch.source().hash() + " with target path" +
> 56: patch.target().path().get())
> 57: );
Is this `get()` safe though? Maybe you want to extract the target path into a variable as well, since the
`patch.target().path().get()` is used a few times below as well? (It would reduce the number of `get()` calls`, so less
scrutiny needed).
webrev/src/main/java/org/openjdk/skara/webrev/Webrev.java line 207:
> 206: " for files " +
> 207: String.join(", ", files.stream().map(Path::toString).collect(Collectors.toList()));
> 208: log.fine("Generating webrev from " + tailEnd + " to " + headHash + filesDesc);
You could also use
[`Collectors.joining`](https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/util/stream/Collectors.html#joining(java.lang.CharSequence))
here Suggestion:
files.stream().map(Path::toString).collect(Collectors.joining(", "));
-------------
PR: https://git.openjdk.java.net/skara/pull/674
More information about the skara-dev
mailing list