RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time

[Roger Riggs]

Hi Nadeesh,

Looks good

Thanks, Roger


On 3/29/2016 8:12 AM, nadeesh tv wrote:
> Hi Roger,
> Please see the updated webrev 
> http://cr.openjdk.java.net/~ntv/8030864/webrev.08/
> Regards,
> Nadeesh
> On 3/11/2016 9:19 PM, Roger Riggs wrote:
>> 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.tv at 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.tv at 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
>>>>>>>
>>>>>
>>>>
>>>
>>
>
> -- 
> Thanks and Regards,
> Nadeesh TV
>