Hi Stephen,
Confirmed, the reviews are in progress to update the spec for 9 and
fix the implementation in 8.
Thanks, Roger
On 12/21/2015 2:05 PM, Stephen Colebourne wrote:
On 11 December 2015 at 20:07, Roger Riggs <roger.ri...@oracle.com> wrote:
Stephen, can you confirm that the added text and test in DateTimeFormatter
is not a specification change?
I thought I replied to this earlier, but maybe not.
This is not a change to the spec, but a clarification to add a clear
spec in an area where clarity was missed. One could argue that some
aspects of this behaviour is implicit from other parts of the API
however it would probably be tricky to tie it up completely.
I have no problem with not porting the spec addition to JDK 8, the but
fix really needs to go back. Not being able to round trip a
ZonedDateTime is really serious, and I remain amazed that this issue
managed to slip through.
Stephen
Our processes have a bit more to do if it is a spec change and it would
limit the backport to JDK 8.
This bug fix will cause an existing TCK/JCK test to fail but that is
considered also a bug and is fixed.
test/java/time/tck/java/time/TCKZonedDateTime.java
Ramanand, some comments on the new test:
- The 'private' qualifier on the tests and data providers is not used in
the current tests and
is not consistently present in the new one.
Since it has little/no function, the tests would be a bit cleaner
without it.
- Typically, test that have data dependencies (such as the timezone data)
cannot be used for
compatibility to the specification, but the data is stable and it seems
unavoidable in this case.
- Are all of the data cases necessary to validate the specification?
Redundant cases extend the testing time without adding more confidence to
the quality.
Thanks, Roger
On 12/10/2015 11:00 AM, Stephen Colebourne wrote:
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.
