Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13528 )
Change subject: [java] Add PartialRow addObject API ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/13528/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java: http://gerrit.cloudera.org:8080/#/c/13528/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@926 PS1, Line 926: * Type.BOOL -> java.lang.Boolean : * Type.INT8 -> java.lang.Byte : * Type.INT16 -> java.lang.Short : * Type.INT32 -> java.lang.Integer : * Type.INT64 -> java.lang.Long : * Type.UNIXTIME_MICROS -> java.sql.Timestamp or java.lang.Long : * Type.FLOAT -> java.lang.Float : * Type.DOUBLE -> java.lang.Double : * Type.STRING -> java.lang.String : * Type.BINARY -> byte[] or java.lang.ByteBuffer : * Type.DECIMAL -> java.math.BigDecimal > More type lists to keep up-to-date; are they useful enough to warrant exist I think so. Given this is public API and these Javadocs are a way for users to understand the expected behavior. http://gerrit.cloudera.org:8080/#/c/13528/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@978 PS1, Line 978: } else { > Given the nesting in this function, would prefer a return after setNull() r Done -- To view, visit http://gerrit.cloudera.org:8080/13528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I046de9dba8f4bedae02b82632e26c10c313c775c Gerrit-Change-Number: 13528 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Thu, 06 Jun 2019 12:47:33 +0000 Gerrit-HasComments: Yes