Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11278 )
Change subject: IMPALA-7466: Improve readability of describe authorization tests ...................................................................... Patch Set 1: (11 comments) http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2694 PS1, Line 2694: private class DescribeOutput { make it a static class http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2695 PS1, Line 2695: private String[] excludedStrings_; : private String[] includedStrings_; initialize these two with empty arrays so there's no need for null checks http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2697 PS1, Line 2697: private TDescribeOutputStyle outputStyle_; make it final http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2703 PS1, Line 2703: nit: no space http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2703 PS1, Line 2703: public DescribeOutput excludeStrings(String [] excluded) { add javadoc http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2708 PS1, Line 2708: nit: no space http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2708 PS1, Line 2708: public DescribeOutput includeStrings(String [] included) { add javadoc http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2721 PS1, Line 2721: nit: no space for consistency with the rest of formatting in this file http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2728 PS1, Line 2728: nit: no space for consistency with the rest of formatting in this file http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2736 PS1, Line 2736: private DescribeOutput describeOutput(TDescribeOutputStyle style) { can be made static http://gerrit.cloudera.org:8080/#/c/11278/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2820 PS1, Line 2820: nit: fix indentation -- To view, visit http://gerrit.cloudera.org:8080/11278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6244b6dbafbd3827d488588a16e66dbf15d0e115 Gerrit-Change-Number: 11278 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 21 Aug 2018 04:42:23 +0000 Gerrit-HasComments: Yes