Looks fine.
thanks!
-Sherman
On 11/05/2013 03:21 PM, Aleksej Efimov wrote:
Sherman,
Thank you for a quick review. I totally agree with you on all items.
Actually, I missed fact that the transitions are sorted. And yes - the change
can be done on line #431.
The new tested fix can be found here:
http://cr.openjdk.java.net/~aefimov/8027848/webrev.01/
<http://cr.openjdk.java.net/%7Eaefimov/8027848/webrev.01/>
-Aleksej
On 11/05/2013 10:58 PM, Xueming Shen wrote:
On 11/05/2013 10:50 AM, Xueming Shen wrote:
Aleksej,
For better performance
(1) the currT should be "static final" so we dont have to access the
System.curentTimeMillis() for each TimeZone/ZoneInfo instance.
(2) instead of iterating through the standardTransitions(), shouldn't we just
check the last one? given it's a sorted list.
and maybe this willGMTOffsetChange setting can be done just at line#431.
-Sherman
btw, in theory, the change now uses the "current vm starttime" instead of
the "tzdb generated" time. But it should be fine, given ZoneInfo.getRawOffset()
will just do a search for the current rawoffset. I may consider to add the
"generated time" into the tzdb.dat in the future, if desired.
Thanks!
-Sherman
On 11/05/2013 09:26 AM, Aleksej Efimov wrote:
Hi,
Can I have a review for a 8027848 [1] bug fix . There is unimplemented
functionality related to the future GMT offset changes.
The ZoneInfoFile class doesn't analyses if there is such offset changes and as
a result incorrectly creates the ZoneInfo instance.
It was discovered during the TestZoneInfo310 test execution as part of
tzdata2013h update [3].
Thanks,
Aleksej
[1] The bug: https://bugs.openjdk.java.net/browse/JDK-8027848
[2] Proposed fix: http://cr.openjdk.java.net/~aefimov/8027848/webrev.00
[3] tzdata2013h review thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/022898.html
