I believe this is suitable for committing, thanks, other reviews welcome! Stephen
On 10 December 2015 at 15:36, Ramanand Patil <ramanand.pa...@oracle.com> wrote: > Hi all, > > Please review the updated webrev: > http://cr.openjdk.java.net/~aefimov/8066982/webrev.01/ > > I have modified the fix and test cases as per inputs given by Stephen. Also, > I have added the javadocs changes in this patch which were proposed in the > bug. > > Bug link is: https://bugs.openjdk.java.net/browse/JDK-8066982 > > > Regards, > Ramanand. > > -----Original Message----- > From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Wednesday, December 09, 2015 4:46 PM > To: core-libs-dev > Cc: i18n-dev > Subject: Re: <i18n dev> Review request for JDK-8066982: ZonedDateTime.parse() > returns wrong ZoneOffset around DST fall transition > > The logic looks fine. > > In the main code, this part > .getLong(INSTANT_SECONDS); > can be replaced with > .toEpochSecond(); > which will be slightly faster. > > In the test case, this part > .plus(15, ChronoUnit.MINUTES); > can be replaced with > .plusMinutes(15) > > And > .with(ChronoField.OFFSET_SECONDS, > ZoneOffset.of(offsetSamples[j]).getTotalSeconds()) > can be replaced with > .with(ZoneOffset.of(offsetSamples[j])) > > In addition to the looping tests, I'd like to see the examples from the bug > report as test cases. Those tests would be simple to follow and explain, > whereas the looping tests are a little hard to follow. > > thanks for fixing this > Stephen > > > > On 9 December 2015 at 07:44, Ramanand Patil <ramanand.pa...@oracle.com> wrote: >> HI all, >> >> >> >> Please review a fix for Bug - HYPERLINK >> "https://bugs.openjdk.java.net/browse/JDK-8066982"JDK-8066982 >> >> >> >> Bug - Parsing a string with ZonedDateTime.parse() that contains zone offset >> and zone ID "Europe/Berlin" returns a wrong ZonedDateAndTime (different >> offset). This error starts exactly at the transition time (included) and >> ends one hour later (excluded). >> >> >> >> Webrev - http://cr.openjdk.java.net/~aefimov/8066982/webrev.00/ >> >> >> >> One existing test case in TCKZonedDateTime.java is also modified, because - >> when offset is invalid the local time is changed to make the result valid. >> >> >> >> >> >> Regards, >> >> Ramanand.
