Todd Lipcon has posted comments on this change.

Change subject: [java-client] Update protoc and simplify the maven build
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6846/5/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java:

Line 266:       // We have UnsafeByteOperations, but that only supports an 
entire array. Here
this actually doesn't appear to be true -- UnsafeByteOperations supports 
unsafeWrap() with an offset and length. Still should be fine to do in a 
follow-up patch but at least the TODO can be made more accurate


http://gerrit.cloudera.org:8080/#/c/6846/5/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:

Line 326:       bytes = value;
don't we also need to check whether length == value.length?

maybe this isn't really worth it after all since this seems to only be used 
when deserializing schemas, which isn't that common an operation in the Java 
client (sorry for leading you astray)


-- 
To view, visit http://gerrit.cloudera.org:8080/6846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I70f7ad260777d5355497fa7e9a1047c342ff9ee9
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <granthe...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Grant Henke <granthe...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to