Adam Holley 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 Can't because of authzFrontend_. 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 Done 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 Done 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 Done 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 Done 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 Done 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 Done 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 Done 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 Done 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 Can't because DescribeOuput will not be 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 Done -- 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: 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 14:16:49 +0000 Gerrit-HasComments: Yes
