Hi Shanliang,
If there's still time - you might want to add the following
comment too:
1353 try {
// The server may have closed the
// connection, expecting us to reconnect.
// The communicatorAdmin will handle
// this case.
1354 communicatorAdmin.gotIOException(ioe);
1355 // reconnection OK, back to "while" to do again
No need to generate a new webrev!
best,
-- daniel
On 9/11/14 3:58 PM, shanliang wrote:
Jaroslav Bachorik wrote:
Hi,
On 09/11/2014 12:31 PM, Daniel Fuchs wrote:
On 9/10/14 9:45 PM, shanliang wrote:
Oh, not one retry attempt fetching the next batch of notifications, but
the *SAME* batch of notifications.
http://cr.openjdk.java.net/~sjiang/JDK-8049303/02/
<http://cr.openjdk.java.net/%7Esjiang/JDK-8049303/02/>
Shanliang
This looks good Shanliang!
I have just one nit - rename "throwsDeserializationException()" to
"rethrowDeserializationException()" - it makes its purpose clear.
I have already added internal comments to explain the call, but why not.
Otherwise - Thumbs Up!
Thanks for review!
Shanliang
Cheers,
-JB-
Make sure to rerun all the JCK/JDK tests before pushing.
This was really a tricky problem!
best regards,
-- daniel
