Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties
File java/kudu-backup/src/test/resources/log4j2.properties:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties@19
PS1, Line 19: name = PropertiesConfig
> Nit:
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties@25
PS1, Line 25: %
> Why is there an isolated open bracket here?
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
File 
java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@42
PS1, Line 42:
> Why is this layout in the capturing appender though? It's not logging that
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@50
PS1, Line 50:     appended.append(LAYOUT.toSerializable(event));
> Can you annotate the meaning of null and true?
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
File 
java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@74
PS1, Line 74:     try (OutputStream os = createOutputStream(useGzip)) {
> Annotate null and true.
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@99
PS1, Line 99:         // ignored
            :       }
            :       outputFileWriter = null;
            :     }
            :     if
> Isn't this the same as IOUtils.closeQuietly?
Yes it is, but IOUtils.closeQuietly is deprecated and the deprecated message 
indicates to use this style.

https://commons.apache.org/proper/commons-io/javadocs/api-release/org/apache/commons/io/IOUtils.html#closeQuietly-java.io.Closeable...-



--
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 13 May 2019 18:02:55 +0000
Gerrit-HasComments: Yes

Reply via email to