Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14736 )
Change subject: Fixed Type.getTypeForName method. Original version uses only name() method to check Type name. Output from name() (inerrithed from Enum) method was different from output from Type.getName() method (ie INT32 vs int32), so the check fails. ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/14736/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14736/1//COMMIT_MSG@7 PS1, Line 7: Fixed Type.getTypeForName method. Nit: Add [java] and a line break after the subject http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/main/java/org/apache/kudu/Type.java File java/kudu-client/src/main/java/org/apache/kudu/Type.java: http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/main/java/org/apache/kudu/Type.java@197 PS1, Line 197: public static Type getTypeForName(String name) { Can you add javadoc that notes that this accepts the ENUM name or the type name? http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/main/java/org/apache/kudu/Type.java@199 PS1, Line 199: if (t.name().equals(name) || t.getName().equals(name)) { Would it make sense to do a case insensitive check by using toLower or toUpper? http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/test/java/org/apache/kudu/TestType.java File java/kudu-client/src/test/java/org/apache/kudu/TestType.java: http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/test/java/org/apache/kudu/TestType.java@37 PS1, Line 37: String name = Type.INT64.getName(); Can you also verify that `Type.INT64.name()` works too? http://gerrit.cloudera.org:8080/#/c/14736/1/java/kudu-client/src/test/java/org/apache/kudu/TestType.java@39 PS1, Line 39: nit: whitespace -- To view, visit http://gerrit.cloudera.org:8080/14736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd0f4f614125d630d128c36fce05f22e19c60100 Gerrit-Change-Number: 14736 Gerrit-PatchSet: 1 Gerrit-Owner: Michele Milesi <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 18 Nov 2019 19:55:22 +0000 Gerrit-HasComments: Yes
