Hi Nadeesh,
Thanks for filling in the missing DateTimeException cases.
- src/java.base/share/classes/java/time/chrono/IsoChronology.java:
" * @throws DateTimeException if the value of any field is out of range,
+ * or if the day-of-month is invalid for the month-of-year"
refers to 'fields' but the values being checked are arguments.
Perhaps: * @throws DateTimeException if the value of any argument is
out of range,
+ * or if the day-of-month is invalid for the
month-of-year
- test/java/time/tck/java/time/chrono/TCKChronology.java:
The new test_bad_epochSecond has unused code:
+ ChronoLocalDate chronoLd = chrono.date(y, m, d);
If y, m, or d were out of range chrono.date would throw the expected
exception instead of epochSecond.
- test/java/time/tck/java/time/chrono/TCKIsoChronology.java
Since IsoChronology has completely different implementation,
test_epochSecond_bad() should
include out of range values for each or m,d,h,m,s.
Thanks, Roger
On 3/10/2016 4:53 AM, nadeesh tv wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8030864/webrev.07/
Changes:
+ @throws DateTimeException if any of the values are out of range
in Chronology.epochSecond()
and
new test cases related to excepted exception in
TCKChronology.test_bad_epochSecond()
Thanks and Regards,
Nadeesh TV
On 3/8/2016 4:14 AM, Roger Riggs wrote:
Look fine.
Roger
On 3/5/2016 7:05 AM, nadeesh tv wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8030864/webrev.06/
Regards,
Nadeesh
On 3/4/2016 4:34 PM, Stephen Colebourne wrote:
long DAYS_0000_TO_1970 should be extracted as a private static
final constant.
Otherwise looks good.
Stephen
On 3 March 2016 at 18:54, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi,
Roger - Thanks for the comments
Made the necessary changes in the spec
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8030864/webrev.05/
On 3/3/2016 12:21 AM, nadeesh tv wrote:
Hi ,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8030864/webrev.03/
Thanks and Regards,
Nadeesh
On 3/3/2016 12:01 AM, Roger Riggs wrote:
Hi Nadeesh,
Editorial comments:
Chronology.java: 716+
"Java epoch" -> "epoch"
"minute, second and zoneOffset" -> "minute, second*,* and
zoneOffset"
(add a comma; two places)
"caluculated using given era, prolepticYear," -> "calculated
using the
era, year-of-era,"
"to represent" -> remove as unnecessary in all places
IsoChronology:
"to represent" -> remove as unnecessary in all places
These should be fixed to cleanup the specification.
The implementation and the tests look fine.
Thanks, Roger
On 3/2/2016 10:17 AM, nadeesh tv wrote:
Hi,
Stephen, Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8030864/webrev.02/
Regards,
Nadeesh TV
On 3/2/2016 5:41 PM, Stephen Colebourne wrote:
Remove "Subclass can override the default implementation for a
more
efficient implementation." as it adds no value.
In the default implementation of
epochSecond(Era era, int yearofEra, int month, int dayOfMonth,
int hour, int minute, int second, ZoneOffset zoneOffset)
use
prolepticYear(era, yearOfEra)
and call the other new epochSecond method. See dateYearDay(Era
era,
int yearOfEra, int dayOfYear) for the design to copy. If this
is done,
then there is no need to override the method in IsoChronology.
In the test,
LocalDate.MIN.with(chronoLd)
could be
LocalDate.from(chronoLd)
Thanks
Stephen
On 2 March 2016 at 10:30, nadeesh tv <nadeesh...@oracle.com>
wrote:
Hi all,
Please review an enhancement for a garbage free epochSecond
method.
Bug ID: https://bugs.openjdk.java.net/browse/JDK-8030864
webrev: http://cr.openjdk.java.net/~ntv/8030864/webrev.01
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
