Review Request - JDK-6720349: (ch) Channels tests depending on hosts inside Sun

[Alan Bateman]

On 06/11/2012 16:10, Daniel Fuchs wrote:
> Hi,
>
> Here is a new webrev incorporating Alan's feedback.
>
> I fixed the indenting issues by looking at the original code
> in the modified file (or in other files in the same directory),
> and trying to guess how the author might have indented the lines
> in question. That's just guesses on my part but hopefully it's
> better now than before.
>
> http://cr.openjdk.java.net/~dfuchs/JDK-6720349/webrev.01/
>
> best regards,
>
> -- daniel
I think this looks very good. The indenting was a bit of a side issue 
anything, I think it's fine now.

On "TestClass", I just realized I suggested this to you in the first 
mail. My brain was thinking "TestServers", my fingers wrote "TestClass" 
for some reason and I see you've pulled it out of TestUtil with that name.

Minor nits:

- in LocalAddress the comment reads "... hopefully it doesn't matter for 
the test purposes". I think this can be removed as an echo server is 
fine here.

- typo in the comment in ConnectState.java (line 114). It reads " We may 
get a an AlreadyConnected exception", I assume this should be "We may 
get AlreadyConnectedException".

Otherwise, thumbs up from me.

-Alan



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20121106/de9cd496/attachment.html