Dan Burkert has posted comments on this change. Change subject: [java-client] separating Connection ......................................................................
Patch Set 13: (9 comments) http://gerrit.cloudera.org:8080/#/c/7146/13//COMMIT_MSG Commit Message: Line 11: The TabletClient has been renamed into RpcHelper. Also, update 'RpcHelper' http://gerrit.cloudera.org:8080/#/c/7146/9/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: Line 151: /** > Just a collection of object representing connections to the servers, no any Hmm so I don't think the new comment is adding much clarity. I think i'd prefer the original, minus the word 'trivial'. My question about 'trivial' is that it suggests the cache is deficient or not-fully feature in some way, but AFAICT it's exactly what we need :) Line 170: > IntellyJIdea was pointing at this as a broken link. This is relative to where you have set up the root IntelliJ project. I'd prefer it you leave it as is, since it's also possible to configure IntelliJ relative to the repo root. http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 74: * a response is currently awaited, as well as temporarily buffered RPCs that are waiting > It seems this comment is stale: there is no need to care about Netty channe Great! Line 598: > Done -- I ended up calling it 'READY'. Good name. http://gerrit.cloudera.org:8080/#/c/7146/13/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 479: * @return true iff the connection is in the READY state. omit the trailing period http://gerrit.cloudera.org:8080/#/c/7146/13/java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java: Line 51: * @{link Connection} objects. I think the @ needs to go inside the opening brace Line 61: /** The logger to use. */ This comment can be omitted, it's obvious from context Line 73: * @param client top-level Kudu client object. omit trailing periods on param docs -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
