-----------------------------------------------------------
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
> 
>

Reply via email to