Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )
Change subject: [java] Upgrade to log4j 2 ...................................................................... Patch Set 1: (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: name = PropertiesConfig 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? It's in other log4j2.properties files too. 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: // This is the standard layout used in Kudu tests. Why is this layout in the capturing appender though? It's not logging that anyone is going to read. 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: super("CapturingLogAppender", null, LAYOUT, true, Property.EMPTY_ARRAY); Can you annotate the meaning of null and true? 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: super("CapturingToFileLogAppender", null, LAYOUT, true, Property.EMPTY_ARRAY); Annotate null and true. 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: try { : outputFileWriter.close(); : } catch (final IOException ioe) { : // ignored : } Isn't this the same as IOUtils.closeQuietly? -- 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: 1 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: Fri, 10 May 2019 21:31:19 +0000 Gerrit-HasComments: Yes
