----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27244/#review62605 -----------------------------------------------------------
src/java/main/org/apache/zookeeper/ClientCnxnSocket.java <https://reviews.apache.org/r/27244/#comment104673> nit: it's implicit that you are talking about the client and clientTunneledAuthenticationInProgress is way too long, how about: tunneledAuthInProgres ? Or maybe something even shorter if you have a better idea. src/java/main/org/apache/zookeeper/ClientCnxnSocket.java <https://reviews.apache.org/r/27244/#comment104674> nit: style, i think we always put the || operator at the end of the expression, i.e.: ``` if (outgoingQueue.getFirst().bb != null || !tunneledAuthInProgress) { } ``` src/java/main/org/apache/zookeeper/ClientCnxnSocket.java <https://reviews.apache.org/r/27244/#comment104677> nit: * extrapolation reads better than concatenation * p.toString() is cheap, so i don't think the isDebugEnabled() fencing is needed here src/java/main/org/apache/zookeeper/ClientCnxnSocket.java <https://reviews.apache.org/r/27244/#comment104675> nit: i'd drop this comment src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/27244/#comment104679> nit, simpler message: ``` throw new IOException("Channel exists: duplicate connect?"); ``` src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/27244/#comment104681> nit: code is straightforward, i'd drop the comment src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/27244/#comment104683> nit: newline src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/27244/#comment104684> nit: drop the comment src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/27244/#comment104687> we are already in a file called netty, lets just call this method cleanup(). - Raul Gutierrez Segales On Nov. 11, 2014, 8:54 p.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27244/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2014, 8:54 p.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-2069 > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/ClientCnxn.java b4ece07 > src/java/main/org/apache/zookeeper/ClientCnxnSocket.java 5ca0ba7 > src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java adb27ee > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION > src/java/main/org/apache/zookeeper/ZooKeeper.java dd13cc9 > src/java/test/org/apache/zookeeper/test/NettyNettySuiteBase.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/NettyNettySuiteHammerTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/test/NettyNettySuiteTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/27244/diff/ > > > Testing > ------- > > > Thanks, > > Hongchao Deng > >
