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