Hi Alan,
Thank you for the feedback.
I have updated the webrev.
http://cr.openjdk.java.net/~ryadav/webrev_8245307/index.html
- rahul
On 12/06/2020 16:57, Alan Bateman wrote:
On 12/06/2020 15:56, Rahul wrote:
Hello,
Request to have my fix reviewed for the issue:
JDK-8245307 : Update ExchangeImpl to use thread safe DateTimeFormatter.
The fix updates sun.net.httpserver.ExchangeImpl to use thread safe
DateTimeFormatter for response headers, this replaces DateFormat
that was using ThreadLocal to be thread safe.
Issue: https://bugs.openjdk.java.net/browse/JDK-8245307
webrev: http://cr.openjdk.java.net/~ryadav/webrev_8245307/index.html
Thanks for taking this one. Implementation change is fine. The test
seems to be quite good but I have small comments:
1. @modules lists several internal packages that don't seem to be
used, maybe copied from another test?
2. It might be easier for reviewers to move the @AfterTest to after
the @BeforeTest so that the setup/tear-down are close together.
3. Are you re-order the declarations so that the instance fields are
together (the static and instance fields are mixed up so slow to read).
4. Minor nit is that you are missing a space in several "if(" and
"try(" usages, maybe an old habit?
-Alan
