Re: FW: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch
Hi Vivek, IMHO, assigning back to methodHandle on line 109, 115, 122,123 is confusing Regards, Nadeesh On 19/05/18 3:07 AM, Vivek Theeyarath wrote: Hi All, A gentle reminder seeking review. Regards Vivek From: Vivek Theeyarath Sent: Thursday, May 17, 2018 6:22 AM To: core-libs-dev@openjdk.java.net Subject: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch Hi All, Please review fix for https://bugs.openjdk.java.net/browse/JDK-8177276 . http://cr.openjdk.java.net/~vtheeyarath/8177276/webrev.02/ The related CSR is https://bugs.openjdk.java.net/browse/JDK-8202991 . Regards Vivek
Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution
Hi Sergei, I could see you modified tests only in /test/java/time/*test*/java/time/format/ directory. Won't the tests from test/java/time/*tck*/java/time/format/ directory fail with same issue? Thanks and Regards, Nadeesh On 12/26/2016 8:27 PM, Sergei Kovalev wrote: Hello Team, Please review below fix for tests. Bug ID: https://bugs.openjdk.java.net/browse/JDK-8171958 Web review: http://cr.openjdk.java.net/~skovalev/8171958/webrev.00/ Issue: some tests fails in case of module limitation by '--limit-module java.base' command line option. Root cause: The tests uses locale data that stored in separate resource file "jdk.localedata". Solution: Add declaration of required module. In same cases a test file contains many test items, part of which could be executed with java.base module only. In this cases we can separate the items and extract that items which depend on locale data and run them individually. Therefore this proposal contains additional test files where added "WithLocale" suffix which demonstrate dependency on resources. Alternative solution could be add a required module declaration "jdk.localedata" to all files. However this will lead to unnecessary test coverage reduction. -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8145633-Adjacent value parsing not supported for Localized Patterns
Hi Roger & Stephen,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8145633/webrev.13/
On 12/21/2016 3:11 AM, Roger Riggs wrote:
Hi Nadeesh,
On 12/20/2016 2:34 PM, nadeesh tv wrote:
Hi Roger & Stephen ,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8145633/webrev.12/
Changes included :
1. Doc changes and cosmetic changes suggested by Roger (except the
note about delay..)
The comments at 4776-4779 are fine.
The issue came from passing null at line 4809 to the
NumberPrinterParser constructor
that expects the field argument to be non-null, and wanting some
explanation of that disconnect.
Changes included: Added the following clarification.
4775 * Prints or parses a localized pattern from a localized field.
4776 * The specific formatter and parameters is not selected until the
4777 * the field is to be printed or parsed.
4778 * The locale is needed to select the proper WeekFields from which
4779 * the field for day-of-week, week-of-month, or week-of-year is
selected.
*4780 * Since Locale is available only during parsing or formatting, the
WeekBasedField
4781 * will be null during construction.*
Is it OK?
Thanks and Regards,
Nadeesh
Other than that, I'm fine with the changes.
Roger
2. Changed the behavior of 'e' to parse only 1 digit as suggested
by Stephen. Changed the existing test cases for this.
Thanks and Regards,
Nadeesh
On 12/20/2016 10:48 PM, Stephen Colebourne wrote:
In the test provider_adjacentValuePatterns2(), please add
{"wwe", Y, w, c, "20161201", 2016, 12, 1},
This should succeed, because a single number is all that is needed to
parse day-of-week. (So, it will need to be removed from the invalid
patterns test).
Line 1869 will need to change to "count, count, count" to make the
tests pass.
Otehrwise, looks fine, thanks.
Stephen
On 20 December 2016 at 09:55, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
BugId: https://bugs.openjdk.java.net/browse/JDK-8145633
Issue: Support adjacent value parsing for Localized Patterns
Webrev: http://cr.openjdk.java.net/~ntv/8145633/webrev.10/
Pattern 'c' and 'W' were previously allowed to have 'zero padding'
which
was not explicitly mentioned in CLDR
(http://unicode.org/reports/tr35/tr35-dates.html).
To allow 'c' and 'W' to take part in adjacent value parsing ( at
the same
time, 2 digits are not required for these patterns), restricted
the max
width of these patterns to 1.
Special thanks to Stephen for the help.
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8145633-Adjacent value parsing not supported for Localized Patterns
Hi Roger & Stephen ,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8145633/webrev.12/
Changes included :
1. Doc changes and cosmetic changes suggested by Roger (except the note
about delay..)
2. Changed the behavior of 'e' to parse only 1 digit as suggested by
Stephen. Changed the existing test cases for this.
Thanks and Regards,
Nadeesh
On 12/20/2016 10:48 PM, Stephen Colebourne wrote:
In the test provider_adjacentValuePatterns2(), please add
{"wwe", Y, w, c, "20161201", 2016, 12, 1},
This should succeed, because a single number is all that is needed to
parse day-of-week. (So, it will need to be removed from the invalid
patterns test).
Line 1869 will need to change to "count, count, count" to make the tests pass.
Otehrwise, looks fine, thanks.
Stephen
On 20 December 2016 at 09:55, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
BugId: https://bugs.openjdk.java.net/browse/JDK-8145633
Issue: Support adjacent value parsing for Localized Patterns
Webrev: http://cr.openjdk.java.net/~ntv/8145633/webrev.10/
Pattern 'c' and 'W' were previously allowed to have 'zero padding' which
was not explicitly mentioned in CLDR
(http://unicode.org/reports/tr35/tr35-dates.html).
To allow 'c' and 'W' to take part in adjacent value parsing ( at the same
time, 2 digits are not required for these patterns), restricted the max
width of these patterns to 1.
Special thanks to Stephen for the help.
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
RFR:JDK-8145633-Adjacent value parsing not supported for Localized Patterns
Hi all, BugId: https://bugs.openjdk.java.net/browse/JDK-8145633 Issue: Support adjacent value parsing for Localized Patterns Webrev: http://cr.openjdk.java.net/~ntv/8145633/webrev.10/ Pattern 'c' and 'W' were previously allowed to have 'zero padding' which was not explicitly mentioned in CLDR (http://unicode.org/reports/tr35/tr35-dates.html). To allow 'c' and 'W' to take part in adjacent value parsing ( at the same time, 2 digits are not required for these patterns), restricted the max width of these patterns to 1. Special thanks to Stephen for the help. -- Thanks and Regards, Nadeesh TV
Re: RFR: JDK-8167618: DateTimeFormatter.format() uses exceptions for flow control.
Hi Roger , Could I push the webrev? http://cr.openjdk.java.net/~ntv/others/anubhav/webrev.01/ <http://cr.openjdk.java.net/%7Entv/others/anubhav/webrev.01/> Thanks and regards, Nadeesh On 11/7/2016 6:05 PM, Anubhav Meena wrote: Thanks for the review! Here is the updated webrev http://cr.openjdk.java.net/~ntv/others/anubhav/webrev.01/ <http://cr.openjdk.java.net/%7Entv/others/anubhav/webrev.01/> -Anubhav On Nov 2, 2016, at 4:04 PM, Stephen Colebourne <scolebou...@joda.org <mailto:scolebou...@joda.org>> wrote: I agree with Nadeesh, the updated code can still throw DateTimeException from the call to getLong(). Unlike before, this DateTimeException is desired. Stephen On 28 October 2016 at 16:58, nadeesh tv <nadeesh...@oracle.com <mailto:nadeesh...@oracle.com>> wrote: Hi Anubhav, - * @throws DateTimeException if the field is not available and the section is not optional I think you should not remove the DTException since still there is a chance of throwing DTE Regards, Nadeesh On 10/28/2016 12:18 AM, Anubhav Meena wrote: Hi all, Please review. Please ignore last mail as links not working properly there. Bug Id : https://bugs.openjdk.java.net/browse/JDK-8167618 <https://bugs.openjdk.java.net/browse/JDK-8167618> Issue: DateTimeFormatter.format() uses exceptions for flow control. Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/ <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8167618/webrev.00/> <http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/ <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8167618/webrev.00/>> -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR 8158880: java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java fail with zh_CN locale
Hi Bhanu,
I think adding Locale to the formatter will be better
For eg:
DateTimeFormatter f =
builder.parseLenient().appendValue(HOUR_OF_DAY).appendValue(MINUTE_OF_HOUR,
2).appendLiteral('9').toFormatter(Locale.UK);
Since other test cases use Locale.UK may be here also you can use UK instead of
locale.US.
Thanks and Regards,
Nadeesh
On 11/11/2016 10:57 AM, Bhanu Gopularam wrote:
Hi Roger,
Thanks for the review. I have updated the test to set default locale (and
restore it) in only those methods which were causing problem in non English
locales.
Please review the updated webrev below:
Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8158880/webrev.01/
Thanks,
Bhanu
From: Roger Riggs
Sent: Tuesday, June 21, 2016 8:36 PM
To: Bhanu Gopularam; core-libs-dev@openjdk.java.net
Cc: Stephen Colebourne
Subject: Re: RFR 8158880:
java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java fail with zh_CN
locale
Hi Bhanu,
It can be problematic to change the default Locale for the entire process,
especially
since many tests are run in the same process. It would be preferable to set
the locale
in the DateTimeFormatter builder instead of changing the process wide Locale.
Please identify the specific test case to apply the fix only where needed.
Is it only
tck.java.time.format.TCKDateTimeFormatterBuilder.test_appendZoneText_parseGenericTimeZonePatterns
where the failure is seen?
This would be an issue testing any pattern letter with locale dependent
behavior.
The locale should be set explicitly in the test.
BTW, There is a predefined Locale.US that can be used, no need to construct a
new Locale.
Roger
On 6/21/2016 6:31 AM, Bhanu Gopularam wrote:
Hi all,
May I request you to review fix for following issue
Bug id: https://bugs.openjdk.java.net/browse/JDK-8158880
Solution: To avoid test failure in non english locales, set the default locale while initial test setup
Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8158880/webrev.00/
Thanks,
Bhanu
--
Thanks and Regards,
Nadeesh TV
Re: RFR 8160036: Java API doc for method minusMonths in LocalDateTime class needs correction
Hi Bhanu, Same issues with OffsetDateTime minusWeeks and minusDays Thanks and Regards, Nadeesh On 11/7/2016 5:31 PM, Bhanu Gopularam wrote: Hi all, Could you please review fix for following issue? Bug id: https://bugs.openjdk.java.net/browse/JDK-8160036 Solution: Corrected documentation for couple of methods in LocalDateTime and OffsetDateTime classes Webrev: http://cr.openjdk.java.net/~bgopularam/8160036/webrev.00 Thanks, Bhanu -- Thanks and Regards, Nadeesh TV
Re: RFR: JDK-8167618: DateTimeFormatter.format() uses exceptions for flow control.
Hi Anubhav, - * @throws DateTimeException if the field is not available and the section is not optional I think you should not remove the DTException since still there is a chance of throwing DTE Regards, Nadeesh On 10/28/2016 12:18 AM, Anubhav Meena wrote: Hi all, Please review. Please ignore last mail as links not working properly there. Bug Id : https://bugs.openjdk.java.net/browse/JDK-8167618 <https://bugs.openjdk.java.net/browse/JDK-8167618> Issue: DateTimeFormatter.format() uses exceptions for flow control. Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/ <http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/> -- Thanks and Regards, Nadeesh TV
Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input
Hi Ivan, ZoneOffset.ofTotalSeconds(Integer.MIN_VALUE) have also the same issue. It' not throwing the expected DTE. May be you can correct this also as part of this. Please update the copyright year also. Rest looks good. -- Thanks and Regards, Nadeesh TV On 8/18/2016 10:23 PM, Ivan Gerasimov wrote: Hello everybody! The factory methods of ZoneOffset class demonstrate a minor inconsistency. Normally, invalid values of arguments are rejected (e.g. when minutes > 59), but the value of Integer.MIN_VALUE is allowed to be passed in. This is due to using Math.abs(), which cannot handle Integer.MIN_VALUE well. Would you please help review the fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8164366 WEBREV: http://cr.openjdk.java.net/~igerasim/8164366/00/webrev/ With kind regards, Ivan -- Thanks and Regards, Nadeesh TV
Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input
Hi , Sorry , my bad. I misread 'plusmn'. -- Thanks and Regards, Nadeesh TV On 8/19/2016 11:10 AM, nadeesh tv wrote: Hi Stephen/Ivan, Is not the the statement "Zone offset minutes and seconds must be negative because hours is negative" and the following doc definition contradicts ? 358 * @param minutes the time-zone offset in minutes, from 0 to 59 359 * @param seconds the time-zone offset in seconds, from 0 to 59 -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi Stephen, Thanks for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8066806/webrev.11/ Changes: Included the suggestions of Stephen Thanks and regards, Nadeesh On 7/22/2016 3:38 PM, Stephen Colebourne wrote: These tests are expected to throw exceptions: test_strict_appendOffsetId() test_strict_appendOffset_1() test_strict_appendOffset_2() test_strict_appendOffset_3() test_strict_appendOffset_4() As such, they should not contain assertEquals(). They should only contain the code that is expected to throw (thus they should not have .get(OFFSET_SECONDS) either). test_strict_offset_adjacentInvalidPattern_parse test_lenient_offset_adjacentInvalidPattern_parse should not have .get(OFFSET_SECONDS) Indentation on line 1621/1622 thanks Stephen On 22 July 2016 at 10:37, nadeesh tv <nadeesh...@oracle.com> wrote: Hi Roger, Thanks for the comments and sorry for the incorrect link. Please see the updated webrev which includes your suggestions. http://cr.openjdk.java.net/~ntv/8066806/webrev.10/ -- Thanks and Regards, Nadeesh TV On 7/21/2016 6:59 PM, Roger Riggs wrote: Hi Nadeesh, Found the changes in http://cr.openjdk.java.net/~ntv/8066806/webrev.09/ Editorial: " In the lenient mode, the parser will be greedy and parse the maximum digits possible." TCKDateTimeFormatterBuilder.java: The lines 1473, 1479, 1485, etc. are way too long, perhaps wrap/break them so each line starts with "." And wrap any other line longer than 100 chars. (Side by side diffs are annoying if the lines are too long). Otherwise, looks good, Thanks, Roger On 7/21/2016 7:21 AM, nadeesh tv wrote: Hi, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8066806/webrev.08/ Changes in this webrev: For leninent mode , doc change in DateTimeFormatterBuilder.java " In the lenient mode, parser will be greedy and parse maximum digits possible. " Added new test cases for lenient mode. -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi Roger, Thanks for the comments and sorry for the incorrect link. Please see the updated webrev which includes your suggestions. http://cr.openjdk.java.net/~ntv/8066806/webrev.10/ -- Thanks and Regards, Nadeesh TV On 7/21/2016 6:59 PM, Roger Riggs wrote: Hi Nadeesh, Found the changes in http://cr.openjdk.java.net/~ntv/8066806/webrev.09/ Editorial: " Inthelenient mode,*the*parser will be greedy and parse*the*maximum digits possible." TCKDateTimeFormatterBuilder.java: The lines 1473, 1479, 1485, etc. are way too long, perhaps wrap/break them so each line starts with "." And wrap any other line longer than 100 chars. (Side by side diffs are annoying if the lines are too long). Otherwise, looks good, Thanks, Roger On 7/21/2016 7:21 AM, nadeesh tv wrote: Hi, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8066806/webrev.08/ Changes in this webrev: For leninent mode , doc change in DateTimeFormatterBuilder.java " In the lenient mode, parser will be greedy and parse maximum digits possible. " Added new test cases for lenient mode. -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.08/
Changes in this webrev:
For leninent mode , doc change in DateTimeFormatterBuilder.java
"
In the lenient mode, parser will be greedy and parse maximum digits possible.
"
Added new test cases for lenient mode.
--
Thanks and Regards,
Nadeesh TV
On 6/10/2016 4:47 PM, nadeesh tv wrote:
Hi,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.08/
Thanks and Regards,
Nadeesh
On 6/9/2016 4:29 PM, nadeesh tv wrote:
Hi Stephen,
On 6/9/2016 4:19 PM, Stephen Colebourne wrote:
"absHours / 10 > 0" would be simpler as "absHours >= 10"
Around line 3595 we have
boolean paddedHour = isPaddedHour();
but 6 lines down isPaddedHour() is used, not the local variable
There is an extra space in the message here:
new DateTimeException(" Value out of Range
also, I'd use "range", not "Range".
Thanks.
The maximum for ZoneOffset is actually 18:00:00 not 23:59:59, however
it is not worth validating that here. The base validation for 23/59/59
that has been added is just fine, values between 18 and 23 will be
rejected later in the processs when attempting to create an instance
of ZoneOffset.
I don't see any tests for
new DateTimeFormatterBuilder().appendOffset(pattern,
"Z").appendValue(ChronoField.NANO_OF_DAY).toFormatter().parse(offset +
"12345")
which should work for most of the patterns.
I intentionally avoided it. I will create a positive test cases for
working patterns and negative test cases for rest.
Regards,
Nadeesh
thanks
Stephen
On 9 June 2016 at 10:27, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi Roger,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.07/
Thanks and regards,
Nadeesh
On 6/9/2016 2:36 AM, Roger Riggs wrote:
HI Nadeesh,
Looking better
DateTimeFormatterBuilder:
- line 3678: If array[1] == 24, offsetSeconds will be greater
that seconds
in a day; that's not right.
I don't think hour=24 is valid. (and there would be test
case(s) for
it.)
There should be test cases for offsets over the limit of hours,
minutes, and
seconds: 24:60:60
Thanks, Roger
On 6/8/2016 2:59 AM, nadeesh tv wrote:
Hi,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.06/
I reused code provided by Stephen and handled the edge cases
accordingly
Thanks and Regards,
Nadeesh
On 5/31/2016 7:15 PM, Stephen Colebourne wrote:
Where the new patterns are described in Javadoc, there is no
discussion of the difference between "H" and "HH".
Add after
"Patterns containing "HH" will format and parse a two digit hour,
zero-padded if necessary. Patterns containing "H" will format with no
zero-padding, and parse either one or two digits."
"with colo" should be "with colon"
As for the main code, I've had a go at a rewrite:
https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4
It is completely untested, and surely has mistakes, however as a
design it seems reasonable.
I agree that the tests need to cover these cases:
- offset at end of line
- offset followed by letters
- offset followed by numbers
Stephen
On 26 May 2016 at 08:49, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please review
BugId : https://bugs.openjdk.java.net/browse/JDK-8066806
Issue: java.time.format.DateTimeFormatter cannot parse an offset
with single
digit hour
webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/
Solution: Added the suggested patterns but the parsing logic became
too
complex.
Appreciate any suggestion to make the parsing less complicated
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8160681:LocalDate.ofEpochDay input validation
Stephen, Roger, Thanks for the comments. On 7/1/2016 7:11 PM, Roger Riggs wrote: Never mind, I just saw the comment in the bug. "Just for a reference, EpochDay range = (LocalDate.MIN.toEpochDay() , LocalDate.MAX.toEpochDay()) " That comment is worth adding to the comments for EpochDay. Please see the updated webrev http://cr.openjdk.java.net/~bgopularam/ntv/8160681/webrev.01/ Thanks and regards, Nadeesh TV Roger On 7/1/2016 9:38 AM, Roger Riggs wrote: Hi Stephen, I'm a bit puzzled by the values recommended for the EpochDay Range. The code should be commented with the computation relative to the range of year MIN/MAX so there is a more complete understanding. I would expect the MIN to be the negative of the MAX or pretty close. Are the new values defined to avoid overflow in some computation? Changing the valid range of values has a (nearly insignificant) compatibility risk. Thanks, Roger On 7/1/2016 8:23 AM, Stephen Colebourne wrote: Fine by me, thanks Stephen On 1 July 2016 at 12:38, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8160681 Issue: Epoch day parameter to LocalDate.ofEpochDay() was not validating Webrev: http://cr.openjdk.java.net/~bgopularam/ntv/8160681/webrev.00/ Tests are already covered under factory_ofEpochDay_aboveMax() , factory_ofEpochDay_belowMin() . Error was obscured. It was throwing DateTimeException because of internally calculated YEAR was going out of range. Now it will throw exception due to expected issue 'epoch day is out of range'. -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8160681:LocalDate.ofEpochDay input validation
Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8160681 Issue: Epoch day parameter to LocalDate.ofEpochDay() was not validating Webrev: http://cr.openjdk.java.net/~bgopularam/ntv/8160681/webrev.00/ Tests are already covered under *f**actory_ofEpochDay_aboveMax()* , *factory_ofEpochDay_belowMin()* . Error was obscured. It was throwing *DateTimeException *because of internally calculated YEAR was going out of range. Now it will throw exception due to expected issue 'epoch day is out of range'. -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.08/
Thanks and Regards,
Nadeesh
On 6/9/2016 4:29 PM, nadeesh tv wrote:
Hi Stephen,
On 6/9/2016 4:19 PM, Stephen Colebourne wrote:
"absHours / 10 > 0" would be simpler as "absHours >= 10"
Around line 3595 we have
boolean paddedHour = isPaddedHour();
but 6 lines down isPaddedHour() is used, not the local variable
There is an extra space in the message here:
new DateTimeException(" Value out of Range
also, I'd use "range", not "Range".
Thanks.
The maximum for ZoneOffset is actually 18:00:00 not 23:59:59, however
it is not worth validating that here. The base validation for 23/59/59
that has been added is just fine, values between 18 and 23 will be
rejected later in the processs when attempting to create an instance
of ZoneOffset.
I don't see any tests for
new DateTimeFormatterBuilder().appendOffset(pattern,
"Z").appendValue(ChronoField.NANO_OF_DAY).toFormatter().parse(offset +
"12345")
which should work for most of the patterns.
I intentionally avoided it. I will create a positive test cases for
working patterns and negative test cases for rest.
Regards,
Nadeesh
thanks
Stephen
On 9 June 2016 at 10:27, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi Roger,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.07/
Thanks and regards,
Nadeesh
On 6/9/2016 2:36 AM, Roger Riggs wrote:
HI Nadeesh,
Looking better
DateTimeFormatterBuilder:
- line 3678: If array[1] == 24, offsetSeconds will be greater that
seconds
in a day; that's not right.
I don't think hour=24 is valid. (and there would be test
case(s) for
it.)
There should be test cases for offsets over the limit of hours,
minutes, and
seconds: 24:60:60
Thanks, Roger
On 6/8/2016 2:59 AM, nadeesh tv wrote:
Hi,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8066806/webrev.06/
I reused code provided by Stephen and handled the edge cases
accordingly
Thanks and Regards,
Nadeesh
On 5/31/2016 7:15 PM, Stephen Colebourne wrote:
Where the new patterns are described in Javadoc, there is no
discussion of the difference between "H" and "HH".
Add after
"Patterns containing "HH" will format and parse a two digit hour,
zero-padded if necessary. Patterns containing "H" will format with no
zero-padding, and parse either one or two digits."
"with colo" should be "with colon"
As for the main code, I've had a go at a rewrite:
https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4
It is completely untested, and surely has mistakes, however as a
design it seems reasonable.
I agree that the tests need to cover these cases:
- offset at end of line
- offset followed by letters
- offset followed by numbers
Stephen
On 26 May 2016 at 08:49, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please review
BugId : https://bugs.openjdk.java.net/browse/JDK-8066806
Issue: java.time.format.DateTimeFormatter cannot parse an offset
with single
digit hour
webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/
Solution: Added the suggested patterns but the parsing logic became too
complex.
Appreciate any suggestion to make the parsing less complicated
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi Roger, Thanks for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8066806/webrev.07/ Thanks and regards, Nadeesh On 6/9/2016 2:36 AM, Roger Riggs wrote: HI Nadeesh, Looking better DateTimeFormatterBuilder: - line 3678: If array[1] == 24, offsetSeconds will be greater that seconds in a day; that's not right. I don't think hour=24 is valid. (and there would be test case(s) for it.) There should be test cases for offsets over the limit of hours, minutes, and seconds: 24:60:60 Thanks, Roger On 6/8/2016 2:59 AM, nadeesh tv wrote: Hi, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8066806/webrev.06/ I reused code provided by Stephen and handled the edge cases accordingly Thanks and Regards, Nadeesh On 5/31/2016 7:15 PM, Stephen Colebourne wrote: Where the new patterns are described in Javadoc, there is no discussion of the difference between "H" and "HH". Add after "Patterns containing "HH" will format and parse a two digit hour, zero-padded if necessary. Patterns containing "H" will format with no zero-padding, and parse either one or two digits." "with colo" should be "with colon" As for the main code, I've had a go at a rewrite: https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4 It is completely untested, and surely has mistakes, however as a design it seems reasonable. I agree that the tests need to cover these cases: - offset at end of line - offset followed by letters - offset followed by numbers Stephen On 26 May 2016 at 08:49, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review BugId : https://bugs.openjdk.java.net/browse/JDK-8066806 Issue: java.time.format.DateTimeFormatter cannot parse an offset with single digit hour webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/ Solution: Added the suggested patterns but the parsing logic became too complex. Appreciate any suggestion to make the parsing less complicated -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8066806/webrev.06/ I reused code provided by Stephen and handled the edge cases accordingly Thanks and Regards, Nadeesh On 5/31/2016 7:15 PM, Stephen Colebourne wrote: Where the new patterns are described in Javadoc, there is no discussion of the difference between "H" and "HH". Add after "Patterns containing "HH" will format and parse a two digit hour, zero-padded if necessary. Patterns containing "H" will format with no zero-padding, and parse either one or two digits." "with colo" should be "with colon" As for the main code, I've had a go at a rewrite: https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4 It is completely untested, and surely has mistakes, however as a design it seems reasonable. I agree that the tests need to cover these cases: - offset at end of line - offset followed by letters - offset followed by numbers Stephen On 26 May 2016 at 08:49, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review BugId : https://bugs.openjdk.java.net/browse/JDK-8066806 Issue: java.time.format.DateTimeFormatter cannot parse an offset with single digit hour webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/ Solution: Added the suggested patterns but the parsing logic became too complex. Appreciate any suggestion to make the parsing less complicated -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi Stephen, Thanks for the suggestions and the code. Regards, Nadeesh On 5/31/2016 7:15 PM, Stephen Colebourne wrote: Where the new patterns are described in Javadoc, there is no discussion of the difference between "H" and "HH". Add after "Patterns containing "HH" will format and parse a two digit hour, zero-padded if necessary. Patterns containing "H" will format with no zero-padding, and parse either one or two digits." "with colo" should be "with colon" As for the main code, I've had a go at a rewrite: https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4 It is completely untested, and surely has mistakes, however as a design it seems reasonable. I agree that the tests need to cover these cases: - offset at end of line - offset followed by letters - offset followed by numbers Stephen On 26 May 2016 at 08:49, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review BugId : https://bugs.openjdk.java.net/browse/JDK-8066806 Issue: java.time.format.DateTimeFormatter cannot parse an offset with single digit hour webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/ Solution: Added the suggested patterns but the parsing logic became too complex. Appreciate any suggestion to make the parsing less complicated -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi ,
On 5/27/2016 7:34 PM, Daniel Fuchs wrote:
On 27/05/16 15:47, Roger Riggs wrote:
Hi Nadeesh,
Seeing the complexity of this code expand, I can't help wonder if there
is a
better algorithm. Perhaps by trying to parse a 1 to 3 numbers(of 1 or
two digits) with optional ":"s.
And then match that to the valid patterns.
The use of numeric indices obscures what is going on.
I will try in this direction.
Also, there are a number of conditions that depend on the length of the
remaining input.
I suspect that when appendOffset is used to parse a sequence of fields,
characters belonging
to the following fields will throw off the checks. I didn't see any
tests that would confirm that
appendOffset with input for additional fields (and input) works as
intended.
Thanks, Roger
Hi,
Stephen is the expert here. However, I can't help feeling
that the patterns that allow you to parse/format a single
digit hour without the ':' separator ("+H" excepted) are
confusing. If we removed those (only adding +H, +H:mm, +H:MM,
+H:MM:ss, +H:MM:SS, +H:mm:ss) would the parsing be simplified?
Would that be an acceptable compromise?
I feel if we remove those patterns for Single Digit hour ( without
colons), it may cause confusion.
Thanks and Regards,
Nadeesh
best regards,
-- daniel
On 5/26/2016 3:49 AM, nadeesh tv wrote:
Hi all,
Please review
BugId : https://bugs.openjdk.java.net/browse/JDK-8066806
Issue: java.time.format.DateTimeFormatter cannot parse an offset with
single digit hour
webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/
Solution: Added the suggested patterns but the parsing logic became
too complex.
Appreciate any suggestion to make the parsing less complicated
--
Thanks and Regards,
Nadeesh TV
RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour
Hi all, Please review BugId : https://bugs.openjdk.java.net/browse/JDK-8066806 Issue: java.time.format.DateTimeFormatter cannot parse an offset with single digit hour webrev: http://cr.openjdk.java.net/~ntv/8066806/webrev.03/ Solution: Added the suggested patterns but the parsing logic became too complex. Appreciate any suggestion to make the parsing less complicated -- Thanks and Regards, Nadeesh TV
Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields
Hi Bhanu, Looking fine to me. Thanks and Regards, Nadeesh On 5/19/2016 4:04 PM, Bhanu Gopularam wrote: Thank you Nadeesh and Stephen. Here is the updated webrev link: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.01 Please review. Bhanu -Original Message- From: Stephen Colebourne [mailto:scolebou...@joda.org] Sent: Tuesday, May 17, 2016 5:11 PM To: core-libs-dev Subject: Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields I would also like to see the test case methods be named "getFrom" not "getfrom". Stephen On 17 May 2016 at 05:18, nadeesh tv <nadeesh...@oracle.com> wrote: Hi Bhanu, I think you should add a test case comparing the return value of getFrom() ( Not an official reviewer) Regards, Nadeesh On 5/16/2016 11:46 AM, Bhanu Gopularam wrote: Hi all, Could you please review fix for following issue. Bug id: https://bugs.openjdk.java.net/browse/JDK-8156718 Solution: Added tck tests for validating getFrom method for unsupported non-Iso temporal fields Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.00/ Thanks, Bhanu -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR 8156718: Need tests for IsoFields getFrom for unsupported non-Iso Temporal fields
Hi Bhanu, I think you should add a test case comparing the return value of getFrom() ( Not an official reviewer) Regards, Nadeesh On 5/16/2016 11:46 AM, Bhanu Gopularam wrote: Hi all, Could you please review fix for following issue. Bug id: https://bugs.openjdk.java.net/browse/JDK-8156718 Solution: Added tck tests for validating getFrom method for unsupported non-Iso temporal fields Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8156718/webrev.00/ Thanks, Bhanu -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8155823: Add date-time patterns 'v' and 'vvvv'
Hi,
Stephen, Roger Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8155823/webrev.04/
Apart from the fix, made a correction in the java doc of ZoneRules.java
Thanks and Regards,
Nadeesh
On 5/10/2016 12:54 AM, Roger Riggs wrote:
Hi Nadeesh,
java/time/format/DateTimeFormatter.java: line 312
- Move 'v' up 1 line so it is after 'V' and before 'z'.
Also in DateTimeFormatterBuilder.java: line 1415+
- the description of the number of letters does not need to be repeated;
put one copy before the specifics of 'z' and 'v'.
"If the count of letters is one, then the short name is output.
+ * If the count of letters is four, then the full name is output.
+ * Two, three and five or more letters throw {@code IllegalArgumentException}."
DateTimeFormatterBuilder.java
- line 1625/1626: should include what happens with vv and vvv to be
consistent with DateTimeFormatter doc for ZoneId names.
- line 1647: The description in DateTimeFormatter says that count = 2
and 3 should also produce the short value
but the code will throw IAE.
-line 1988: "almost" --- can this be more specific about what
matches or does not match
In the test it is a bit bothersome that the tests have to rely on
timezone specific data.
Thanks, Roger
On 5/9/2016 12:35 PM, nadeesh tv wrote:
Hi all,
Reposting because subject line did not follow the format.
Bug Id : https://bugs.openjdk.java.net/browse/JDK-8155823
Issue: Add date-time patterns 'v' and ''
webrev: http://cr.openjdk.java.net/~ntv/8155823/webrev.03/
This webrev also contain the fix of related bug
https://bugs.openjdk.java.net/browse/JDK-8154567 as part of this.
Special thanks for Stephen for his help in writing the spec.
--
Thanks and Regards,
Nadeesh TV
RFR:JDK-8155823: Add date-time patterns 'v' and 'vvvv'
Hi all, Reposting because subject line did not follow the format. Bug Id : https://bugs.openjdk.java.net/browse/JDK-8155823 Issue: Add date-time patterns 'v' and '' webrev: http://cr.openjdk.java.net/~ntv/8155823/webrev.03/ This webrev also contain the fix of related bug https://bugs.openjdk.java.net/browse/JDK-8154567 as part of this. Special thanks for Stephen for his help in writing the spec. -- Thanks and Regards, Nadeesh TV
Add date-time patterns 'v' and 'vvvv'
Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8155823 Issue: Add date-time patterns 'v' and '' webrev: http://cr.openjdk.java.net/~ntv/8155823/webrev.03/ This webrev also contain the fix of related bug https://bugs.openjdk.java.net/browse/JDK-8154567 as part of this. Special thanks for Stephen for his help in writing the spec. -- Thanks and Regards, Nadeesh TV
Add date-time patterns 'v' and 'vvvv'
Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8155823 Issue: Add date-time patterns 'v' and '' webrev: http://cr.openjdk.java.net/~ntv/8155823/webrev.03/ This webrev also contain the fix of related bug https://bugs.openjdk.java.net/browse/JDK-8154567 as part of this. -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value
Hi Roger, Ok. Thanks. Regards, Nadeesh On 5/4/2016 7:58 PM, Roger Riggs wrote: Hi Nadeesh, java/time/format/DateTimeFormatterBuilder.java: 1836-1839 Correct as is, but could collapse both count ==2 and count ==3 into a single appendValue call: appendValue(field, count, 3, SignStyle.NOT_NEGATIVE); Reviewed. Roger On 5/4/2016 3:15 AM, nadeesh tv wrote: Hi, Updated the webev http://cr.openjdk.java.net/~ntv/8079628/webrev.04/ Regards, Nadeesh On 5/3/2016 8:45 PM, Stephen Colebourne wrote: Letters "Q", "q", "M", "L", "d", "D", "F", "h", "H", "k", "K", "m", "s" and no doubt others use NORMAL via appendValue(field); Changing these to use NOT_NEGATIVE would be a big change and doesn't seem justified. For "D", "DD" and "DDD" these seem to be the best balance (as discussed up-thread): *D 1 appendValue(ChronoField.DAY_OF_YEAR) *DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3, SignStyle.NOT_NEGATIVE) *DDD 3 appendValue(ChronoField.DAY_OF_YEAR, 3) ie. we use NOT_NEGATIVE when we are making changes to a field and that is the best option, but otherwise we stick with NORMAL for single letter patterns like "D". Stephen On 3 May 2016 at 15:22, Roger Riggs <roger.ri...@oracle.com> wrote: Hi Nadeesh, On 5/3/2016 3:24 AM, nadeesh tv wrote: Hi Roger, Please see the answers inline On 5/3/2016 2:43 AM, Roger Riggs wrote: Hi Nadeesh, src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 1835: the appendValue(field) method has a sign-style of NORMAL; should that be NOT_NEGATIVE? It's 'NOT_NEGATIVE' only. May be a browser caching issue. Can you please check it once again. My question was about the existing case for "D" that uses appendValue(field). That is a shorthand for: appendValue(new NumberPrinterParser(field, 1, 19, SignStyle.NORMAL)); test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java: What is being tested in test_dayOfYearFieldPattern? It does not check that the value parsed matches the input. These test cases added initially. Now it's become almost redundant due to 'dayOfYearFieldValues' . I could remove it. If they are redundant, they should be removed. test/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 686, 688: should these cases show with sign-style NOT_NEGATIVE also? Suspecting browser caching here also. I'm referring to the test case for "D", which seems like it should show up as NOT_NEGATIVE. Updated the webrev by removing some unnecessary spaces http://cr.openjdk.java.net/~ntv/8079628/webrev.03/ Thanks, Roger -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value
Hi, Updated the webev http://cr.openjdk.java.net/~ntv/8079628/webrev.04/ Regards, Nadeesh On 5/3/2016 8:45 PM, Stephen Colebourne wrote: Letters "Q", "q", "M", "L", "d", "D", "F", "h", "H", "k", "K", "m", "s" and no doubt others use NORMAL via appendValue(field); Changing these to use NOT_NEGATIVE would be a big change and doesn't seem justified. For "D", "DD" and "DDD" these seem to be the best balance (as discussed up-thread): *D 1 appendValue(ChronoField.DAY_OF_YEAR) *DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3, SignStyle.NOT_NEGATIVE) *DDD 3 appendValue(ChronoField.DAY_OF_YEAR, 3) ie. we use NOT_NEGATIVE when we are making changes to a field and that is the best option, but otherwise we stick with NORMAL for single letter patterns like "D". Stephen On 3 May 2016 at 15:22, Roger Riggs <roger.ri...@oracle.com> wrote: Hi Nadeesh, On 5/3/2016 3:24 AM, nadeesh tv wrote: Hi Roger, Please see the answers inline On 5/3/2016 2:43 AM, Roger Riggs wrote: Hi Nadeesh, src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 1835: the appendValue(field) method has a sign-style of NORMAL; should that be NOT_NEGATIVE? It's 'NOT_NEGATIVE' only. May be a browser caching issue. Can you please check it once again. My question was about the existing case for "D" that uses appendValue(field). That is a shorthand for: appendValue(new NumberPrinterParser(field, 1, 19, SignStyle.NORMAL)); test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java: What is being tested in test_dayOfYearFieldPattern? It does not check that the value parsed matches the input. These test cases added initially. Now it's become almost redundant due to 'dayOfYearFieldValues' . I could remove it. If they are redundant, they should be removed. test/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 686, 688: should these cases show with sign-style NOT_NEGATIVE also? Suspecting browser caching here also. I'm referring to the test case for "D", which seems like it should show up as NOT_NEGATIVE. Updated the webrev by removing some unnecessary spaces http://cr.openjdk.java.net/~ntv/8079628/webrev.03/ Thanks, Roger -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'
Hi, Updated the webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.03/ Thanks and Regards, Nadeesh On 5/3/2016 8:37 PM, Stephen Colebourne wrote: The current behaviour is to use NORMAL for "A" and NOT_NEGATIVE for "AA", "AAA" and so on. The sensible behaviour going forward is to use NOT_NEGATIVE for all these, simply because the values do not make sense to be negative. Given how these fields are nigh-on useless as currently defined, this seems reasonable. Stephen On 3 May 2016 at 15:37, Roger Riggs <roger.ri...@oracle.com> wrote: Hi Nadeesh, src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder:1522-1524 Is the switch from SignStyle.NOT_NEGATIVE to NORMAL intentional? The ValueRange of MilliOfDay for example is (0, 8640-1), so negative values would be out of range. Similarly, NanoOfSecond and NanoOfDay are non-negative. (Otherwise, there should be test cases for negative values). Thanks, Roger On 4/28/2016 4:04 PM, nadeesh tv wrote: Hi all, Thanks Stephen for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.02/ Regards, Nadeesh On 4/28/2016 7:58 PM, Stephen Colebourne wrote: I'd like to see the test cases in test_secondsPattern() check the result of the parse (by passing more arguments from data_secondsPattern) Otherwise looks good. Stephen On 28 April 2016 at 14:12, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.01/ Regards, Nadeesh TV On 4/25/2016 8:08 PM, nadeesh tv wrote: HI all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949 Issue - Pattern letters 'A' does not match the intent of LDML/CLDR Solution - Changed the definition of pattern letters 'A','n','N' Webrev - http://cr.openjdk.java.net/~ntv/8148949/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value
Hi Roger, Please see the answers inline On 5/3/2016 2:43 AM, Roger Riggs wrote: Hi Nadeesh, src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 1835: the appendValue(field) method has a sign-style of NORMAL; should that be NOT_NEGATIVE? It's 'NOT_NEGATIVE' only. May be a browser caching issue. Can you please check it once again. test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java: What is being tested in test_dayOfYearFieldPattern? It does not check that the value parsed matches the input. These test cases added initially. Now it's become almost redundant due to 'dayOfYearFieldValues' . I could remove it. test/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 686, 688: should these cases show with sign-style NOT_NEGATIVE also? Suspecting browser caching here also. Updated the webrev by removing some unnecessary spaces http://cr.openjdk.java.net/~ntv/8079628/webrev.03/ Regards, Nadeesh TV Thanks, Roger On 4/28/2016 2:04 PM, nadeesh tv wrote: Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8079628/webrev.02/ Thanks and Regards, Nadeesh TV On 4/28/2016 8:17 PM, Stephen Colebourne wrote: My mistake on the spec for "DD". It should be SignStyle.NOT_NEGATIVE, not NORMAL. I'd like to see a test that checks adjacent value parsing works correctly for "DDD". ie. DDDHHmm should be able to parse into a LocalDateTime where the date-time value can be checked against the input string. I think this will be a good fix once complete. thanks Stephen On 28 April 2016 at 14:10, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Thanks Stephen for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8079628/webrev.01/ Regards, Nadeesh TV On 4/26/2016 5:42 PM, Stephen Colebourne wrote: This change introduces inconsistencies and problems. For example, it will parse up to 19 digits for "D" but only up to 2 digits for "d". The following would be better: *D 1 appendValue(ChronoField.DAY_OF_YEAR) *DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3, SignStyle.NORMAL) *DDD 3 appendValue(ChronoField.DAY_OF_YEAR, 3) This limits the accepted input to 3 digits, which is quite sufficient here. It is also clearer for the common "D" and "DDD" cases. Note that a test case needs to cover adjacent value parsing for day-of-year. This pattern "DDD" should be tested and work. With the webrev changes, it will not work as adjacent value parsing mode will not be triggered. (The change will alter the meaning of "DD" but no-one should be using that anyway as it is broken, only working for the first 99 days of the year.) The code in TCKDateTimeFormatterBuilder needs a blank line after the new methods. Stephen On 26 April 2016 at 12:48, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8079628 Issue - Pattern 'D' is not implemented as intended by CLDR Solution - Changed the definition of 'D' to D..D 1..3 appendValue(ChronoField.DAY_OF_YEAR, n, 19, SignStyle.NORMAL) Webrev - http://cr.openjdk.java.net/~ntv/8079628/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'
Hi all, Thanks Stephen for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.02/ Regards, Nadeesh On 4/28/2016 7:58 PM, Stephen Colebourne wrote: I'd like to see the test cases in test_secondsPattern() check the result of the parse (by passing more arguments from data_secondsPattern) Otherwise looks good. Stephen On 28 April 2016 at 14:12, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.01/ Regards, Nadeesh TV On 4/25/2016 8:08 PM, nadeesh tv wrote: HI all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949 Issue - Pattern letters 'A' does not match the intent of LDML/CLDR Solution - Changed the definition of pattern letters 'A','n','N' Webrev - http://cr.openjdk.java.net/~ntv/8148949/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value
Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8079628/webrev.02/ Thanks and Regards, Nadeesh TV On 4/28/2016 8:17 PM, Stephen Colebourne wrote: My mistake on the spec for "DD". It should be SignStyle.NOT_NEGATIVE, not NORMAL. I'd like to see a test that checks adjacent value parsing works correctly for "DDD". ie. DDDHHmm should be able to parse into a LocalDateTime where the date-time value can be checked against the input string. I think this will be a good fix once complete. thanks Stephen On 28 April 2016 at 14:10, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Thanks Stephen for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8079628/webrev.01/ Regards, Nadeesh TV On 4/26/2016 5:42 PM, Stephen Colebourne wrote: This change introduces inconsistencies and problems. For example, it will parse up to 19 digits for "D" but only up to 2 digits for "d". The following would be better: *D 1 appendValue(ChronoField.DAY_OF_YEAR) *DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3, SignStyle.NORMAL) *DDD 3 appendValue(ChronoField.DAY_OF_YEAR, 3) This limits the accepted input to 3 digits, which is quite sufficient here. It is also clearer for the common "D" and "DDD" cases. Note that a test case needs to cover adjacent value parsing for day-of-year. This pattern "DDD" should be tested and work. With the webrev changes, it will not work as adjacent value parsing mode will not be triggered. (The change will alter the meaning of "DD" but no-one should be using that anyway as it is broken, only working for the first 99 days of the year.) The code in TCKDateTimeFormatterBuilder needs a blank line after the new methods. Stephen On 26 April 2016 at 12:48, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8079628 Issue - Pattern 'D' is not implemented as intended by CLDR Solution - Changed the definition of 'D' to D..D 1..3 appendValue(ChronoField.DAY_OF_YEAR, n, 19, SignStyle.NORMAL) Webrev - http://cr.openjdk.java.net/~ntv/8079628/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'
Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.01/ Regards, Nadeesh TV On 4/25/2016 8:08 PM, nadeesh tv wrote: HI all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949 Issue - Pattern letters 'A' does not match the intent of LDML/CLDR Solution - Changed the definition of pattern letters 'A','n','N' Webrev - http://cr.openjdk.java.net/~ntv/8148949/webrev.00/ -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value
Hi all, Thanks Stephen for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8079628/webrev.01/ Regards, Nadeesh TV On 4/26/2016 5:42 PM, Stephen Colebourne wrote: This change introduces inconsistencies and problems. For example, it will parse up to 19 digits for "D" but only up to 2 digits for "d". The following would be better: *D 1 appendValue(ChronoField.DAY_OF_YEAR) *DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3, SignStyle.NORMAL) *DDD 3 appendValue(ChronoField.DAY_OF_YEAR, 3) This limits the accepted input to 3 digits, which is quite sufficient here. It is also clearer for the common "D" and "DDD" cases. Note that a test case needs to cover adjacent value parsing for day-of-year. This pattern "DDD" should be tested and work. With the webrev changes, it will not work as adjacent value parsing mode will not be triggered. (The change will alter the meaning of "DD" but no-one should be using that anyway as it is broken, only working for the first 99 days of the year.) The code in TCKDateTimeFormatterBuilder needs a blank line after the new methods. Stephen On 26 April 2016 at 12:48, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8079628 Issue - Pattern 'D' is not implemented as intended by CLDR Solution - Changed the definition of 'D' to D..D 1..3 appendValue(ChronoField.DAY_OF_YEAR, n, 19, SignStyle.NORMAL) Webrev - http://cr.openjdk.java.net/~ntv/8079628/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value
Hi all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8079628 Issue - Pattern 'D' is not implemented as intended by CLDR Solution - Changed the definition of 'D' to D..D 1..3 appendValue(ChronoField.DAY_OF_YEAR, n, 19, SignStyle.NORMAL) Webrev - http://cr.openjdk.java.net/~ntv/8079628/webrev.00/ <http://cr.openjdk.java.net/%7Entv/8079628/webrev.00/> -- Thanks and Regards, Nadeesh TV
RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'
HI all, Please review a fix for Bug ID - https://bugs.openjdk.java.net/browse/JDK-8148949 Issue - Pattern letters 'A' does not match the intent of LDML/CLDR Solution - Changed the definition of pattern letters 'A','n','N' Webrev - http://cr.openjdk.java.net/~ntv/8148949/webrev.00/ -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8031085:DateTimeFormatter won't parse dates with custom format "yyyyMMddHHmmssSSS"
Hi Roger,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8031085/webrev.02/
Regards,
Nadeesh TV
On 4/19/2016 10:51 PM, Roger Riggs wrote:
Hi Nadeesh,
java/time/format/DateTimeFormatterBuilder.java:
- line 671, the @code should be @link, especially since it says
"see", to make navigation easier
- line 2998: Missing first sentence of the method description.
otherwise looks ok.
Roger
On 4/19/2016 10:16 AM, Stephen Colebourne wrote:
Looks good.
Stephen
On 19 April 2016 at 09:42, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi Stephen,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8031085/webrev.01/
--
Thanks and Regards,
Nadeesh TV
On 4/18/2016 3:03 AM, Stephen Colebourne wrote:
The updated spec at line 670 is not clear - the adjacent parsing mode
only applies when in strict mode. Suggest a new sentence before the
lenient mode one: "In strict mode, if the minimum and maximum widths
are equal and there is no decimal point then the parser will
participate in adjacent value parsing, see {@code
appendValue(java.time.temporal.TemporalField, int)}.
Line 2968 is missing a blank line
Line 3001 does not need == true on isStrict() == true
Perhaps line 3004 should return false? I'm not sure what is gained by
calling super.
The changes to the test cases look wrong.
test_adjacent_lenient_fractionFollows_2digit() and
test_adjacent_lenient_fractionFollows_0digit() should not have
changed, as the nano-of-second parsing is in lenient mode, and thus
should still parse from zero to nine digits.
thanks
Stephen
On 13 April 2016 at 21:46, nadeesh tv <nadeesh...@oracle.com> wrote:
HI all,
BUG ID : https://bugs.openjdk.java.net/browse/JDK-8031085
Webrev : http://cr.openjdk.java.net/~ntv/8031085/webrev.00/
Issue - Fractional parts of seconds do not participate in the
protocol
for
adjacent value parsing
Solution - Changed the FractionPrinterParser to subclass of
NumberPrinterParser to make it participate in adjacent value parsing
2 existing test cases
TCKDateTimeFormatterBuilder.test_adjacent_lenient_fractionFollows_0digit
and
test_adjacent_lenient_fractionFollows_2digit were failing. Changed
them
accordingly.
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8031085:DateTimeFormatter won't parse dates with custom format "yyyyMMddHHmmssSSS"
Hi Stephen,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8031085/webrev.01/
--
Thanks and Regards,
Nadeesh TV
On 4/18/2016 3:03 AM, Stephen Colebourne wrote:
The updated spec at line 670 is not clear - the adjacent parsing mode
only applies when in strict mode. Suggest a new sentence before the
lenient mode one: "In strict mode, if the minimum and maximum widths
are equal and there is no decimal point then the parser will
participate in adjacent value parsing, see {@code
appendValue(java.time.temporal.TemporalField, int)}.
Line 2968 is missing a blank line
Line 3001 does not need == true on isStrict() == true
Perhaps line 3004 should return false? I'm not sure what is gained by
calling super.
The changes to the test cases look wrong.
test_adjacent_lenient_fractionFollows_2digit() and
test_adjacent_lenient_fractionFollows_0digit() should not have
changed, as the nano-of-second parsing is in lenient mode, and thus
should still parse from zero to nine digits.
thanks
Stephen
On 13 April 2016 at 21:46, nadeesh tv <nadeesh...@oracle.com> wrote:
HI all,
BUG ID : https://bugs.openjdk.java.net/browse/JDK-8031085
Webrev : http://cr.openjdk.java.net/~ntv/8031085/webrev.00/
Issue - Fractional parts of seconds do not participate in the protocol for
adjacent value parsing
Solution - Changed the FractionPrinterParser to subclass of
NumberPrinterParser to make it participate in adjacent value parsing
2 existing test cases
TCKDateTimeFormatterBuilder.test_adjacent_lenient_fractionFollows_0digit and
test_adjacent_lenient_fractionFollows_2digit were failing. Changed them
accordingly.
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset
Hi Roger/Stephen, On 4/18/2016 2:40 AM, Stephen Colebourne wrote: The LDML spec indicates that the "GMT" string should be localized: http://unicode.org/repos/cldr/trunk/specs/ldml/tr35-dates.html#Using_Time_Zone_Names The text of appendLocalizedOffset() is written with the intention of the output being localized (otherwise, what is the point of the method!) I assume this was something that got missed when implementing appendLocalizedOffset(). It may require additional localized data from the LDML data files. If OK, I will create a new enhancement request for this in JBS Regards, Nadeesh This webrev looks fine. Stephen On 13 April 2016 at 16:56, Roger Riggs <roger.ri...@oracle.com> wrote: Hi Nadeesh, The bugfix looks fine. The TODO comment on the "GMT" raises the question (as a separate issue) about implementing the TODO or removing the TODO comment. I'm not sure where the localized string for "GMT" would come from but it might be a useful improvement unless it was judged to a compatibility issue. Roger On 4/13/2016 10:19 AM, nadeesh tv wrote: HI all, Bug Id - https://bugs.openjdk.java.net/browse/JDK-8154050 Issue - java.time.format.DateTimeFormatter can't parse localized zone-offset Solution - Corrected the mistake in calculating parse end position and removed an unnecessary null check webrev - http://cr.openjdk.java.net/~ntv/8154050/webrev.00/ PS: TCKOffsetPrinterParser.test_print_localized() already contain some test cases related to parsing and formatting. therefore did not repeat in the new test cases file -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8031085:DateTimeFormatter won't parse dates with custom format "yyyyMMddHHmmssSSS"
HI all, BUG ID : https://bugs.openjdk.java.net/browse/JDK-8031085 Webrev : http://cr.openjdk.java.net/~ntv/8031085/webrev.00/ Issue - Fractional parts of seconds do not participate in the protocol for adjacent value parsing Solution - Changed the FractionPrinterParser to subclass of NumberPrinterParser to make it participate in adjacent value parsing 2 existing test cases TCKDateTimeFormatterBuilder.test_adjacent_lenient_fractionFollows_0digit and test_adjacent_lenient_fractionFollows_2digit were failing. Changed them accordingly. -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset
Hi all, Please see the updated the webrev with a minor doc change - *O 1 appendLocalizedOffsetPrefixed(TextStyle.SHORT); - *4 appendLocalizedOffsetPrefixed(TextStyle.FULL); + *O 1 appendLocalizedOffset(TextStyle.SHORT); + *4 appendLocalizedOffset(TextStyle.FULL); webrev : http://cr.openjdk.java.net/~ntv/8154050/webrev.01/ On 4/13/2016 10:36 PM, Roger Riggs wrote: Hi Lance, Literal strings are interned by the compiler but it would make it clearer that it is the same string everywhere. Though I find it easier to read when the value is inline without having to do the indirection to a constant. And its really not going to change; "GMT" is too deeply embedded in the nomenclature. Roger On 4/13/2016 12:33 PM, Lance Andersen wrote: On Apr 13, 2016, at 11:56 AM, Roger Riggs <roger.ri...@oracle.com> wrote: Hi Nadeesh, The bugfix looks fine. The TODO comment on the "GMT" raises the question (as a separate issue) about implementing the TODO or removing the TODO comment. I'm not sure where the localized string for "GMT" would come from but it might be a useful improvement unless it was judged to a compatibility issue. Could gmtText be made static final as it is declared in 3 or 4 methods if it is not being localized? Roger On 4/13/2016 10:19 AM, nadeesh tv wrote: HI all, Bug Id - https://bugs.openjdk.java.net/browse/JDK-8154050 <https://bugs.openjdk.java.net/browse/JDK-8154050> Issue - java.time.format.DateTimeFormatter can't parse localized zone-offset Solution - Corrected the mistake in calculating parse end position and removed an unnecessary null check webrev - http://cr.openjdk.java.net/~ntv/8154050/webrev.00/ <http://cr.openjdk.java.net/%7Entv/8154050/webrev.00/> <http://cr.openjdk.java.net/%7Entv/8154050/webrev.00/> PS: TCKOffsetPrinterParser.test_print_localized() already contain some test cases related to parsing and formatting. therefore did not repeat in the new test cases file -- Thanks and Regards, Nadeesh TV <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8148849:Truncating Duration
Hi, I need one more review for this change Regards, Nadeesh On 3/30/2016 7:03 PM, Stephen Colebourne wrote: Yes, that looks OK now. thanks Stephen On 30 March 2016 at 12:25, nadeesh tv <nadeesh...@oracle.com> wrote: Hi Stephen, Thanks for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148849/webrev.01/ Made a change in unit == ChronoUnit.SECONDS also Regards, Nadeesh TV On 3/29/2016 6:10 PM, Stephen Colebourne wrote: We're almost there, but looking at the tests, it looks like the behaviour is wrong: The intended behaviour is that -20.5mins (minus 20 minutes 30 secs) should truncate to -20mins -2.1secs truncate to -2secs Note that the truncation is different to Instant here. An Instant truncates towards the far past - like RoundingMode.FLOOR A Duration truncates towards the zero - like RoundingMode.DOWN Stephen On 29 March 2016 at 13:18, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8148849 Enhanced Duration by adding public Duration truncatedTo(TemporalUnit unit) Please http://cr.openjdk.java.net/~ntv/8148849/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8148947:DateTimeFormatter pattern letter 'g'
Gentle reminder On 3/30/2016 11:41 PM, nadeesh tv wrote: Hi all, BUG ID : https://bugs.openjdk.java.net/browse/JDK-8148947 Webrev : http://cr.openjdk.java.net/~ntv/8148947/webrev.00/ Added new pattern letter 'g' for Modified Julian day. Apart from that made clarification to JulianFields and removed semicolons from docs of DateTimeFormatterBuilder as suggested by Stephen -- Thanks and Regards, Nadeesh TV
RFR:JDK-8148947:DateTimeFormatter pattern letter 'g'
Hi all, BUG ID : https://bugs.openjdk.java.net/browse/JDK-8148947 Webrev : http://cr.openjdk.java.net/~ntv/8148947/webrev.00/ Added new pattern letter 'g' for Modified Julian day. Apart from that made clarification to JulianFields and removed semicolons from docs of DateTimeFormatterBuilder as suggested by Stephen -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8148849:Truncating Duration
Hi Stephen, Thanks for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8148849/webrev.01/ Made a change in unit == ChronoUnit.SECONDS also Regards, Nadeesh TV On 3/29/2016 6:10 PM, Stephen Colebourne wrote: We're almost there, but looking at the tests, it looks like the behaviour is wrong: The intended behaviour is that -20.5mins (minus 20 minutes 30 secs) should truncate to -20mins -2.1secs truncate to -2secs Note that the truncation is different to Instant here. An Instant truncates towards the far past - like RoundingMode.FLOOR A Duration truncates towards the zero - like RoundingMode.DOWN Stephen On 29 March 2016 at 13:18, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8148849 Enhanced Duration by adding public Duration truncatedTo(TemporalUnit unit) Please http://cr.openjdk.java.net/~ntv/8148849/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8148849:Truncating Duration
Hi all, Bug Id : https://bugs.openjdk.java.net/browse/JDK-8148849 Enhanced Duration by adding public Duration truncatedTo(TemporalUnit unit) Please http://cr.openjdk.java.net/~ntv/8148849/webrev.00/ -- Thanks and Regards, Nadeesh TV
RFR:JDK-8148950 :Enhance ChronoField Javadoc
Hi all, Please see the doc changes http://cr.openjdk.java.net/~ntv/8148950/webrev.00/ Bug ID https://bugs.openjdk.java.net/browse/JDK-8148950 -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
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__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 -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
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__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 -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
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__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 -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
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
Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")
M should be allowed to be optional. There is no
question of parsing
extra digits not included in the requested pattern.
Separately, this is specifying the new lenient behavior of
appendOffset(pattern, noffsetText).
In that case, I don't think it will be understood that patterns
'shorter'
than the input will
gobble up extra digits and ':'s.
Roger
On 2/26/2016 9:42 AM, Stephen Colebourne wrote:
Lenient can be however lenient we define it to be. Allowing minutes
and seconds to be parsed when not specified in the pattern is the key
part of the change. Whether the parser copes with both colons and
no-colons is the choice at hand here. It seems to me that since the
parser can easily handle figuring out whether the colon is present or
not, we should just allow the parser to be fully lenient.
Stephen
On 26 February 2016 at 14:15, Roger Riggs <roger.ri...@oracle.com>
wrote:
HI Stephen,
How lenient is lenient supposed to be? Looking at the offset test
cases,
it
seems to allow minutes
and seconds digits to be parsed even if the pattern did not include
them.
+ @DataProvider(name="lenientOffsetParseData")
+ Object[][] data_lenient_offset_parse() {
+ return new Object[][] {
+ {"+HH", "+01", 3600},
+ {"+HH", "+0101", 3660},
+ {"+HH", "+010101", 3661},
+ {"+HH", "+01", 3600},
+ {"+HH", "+01:01", 3660},
+ {"+HH", "+01:01:01", 3661},
Thanks, Roger
On 2/26/2016 6:16 AM, Stephen Colebourne wrote:
I don't think this is quite right.
if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
parseType = 10;
}
This code will *always* select type 10 (colons) if a colon is found at
position+3. Whereas the spec now says that it should only do this if
the pattern is "HH". For other patterns, the colon/no-colon choice is
defined to be based on the pattern.
That said, I'm thinking it is better to make the spec more lenient to
match the behaviour as implemented:
When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional. If the character after the hour digits is a
colon
then the parser will parse using the pattern "HH:mm:ss", otherwise the
parser will parse using the pattern "HHmmss".
Additional TCKDateTimeFormatterBuilder tests will be needed to
demonstrate the above. There should also be a test for data following
the lenient parse. The following should all succeed:
DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
"+01:00Europe/London"
"+0100Europe/London"
DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
"+01:Europe/London"
Note this special case, where the colon affects the parse type, but is
not ultimately part of the offset, thus it is left to match the
appendLiteral(":")
You may want to think of some additional nasty edge cases!
Stephen
On 25 February 2016 at 15:44, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
Thanks and Regards,
Nadeesh
On 2/23/2016 5:17 PM, Stephen Colebourne wrote:
Thanks for the changes.
In `DateTimeFormatter`, the code should be
.parseLenient()
.appendOffsetId()
.parseStrict()
and the same in the other case. This ensures that existing callers who
then embed the formatter in another formatter (like the
ZONED_DATE_TIME constant) are unaffected.
The logic for lenient parsing does not look right as it only handles
types 5 and 6. This table shows the mappings needed:
"+HH", -> "+HHmmss" or "+HH:mm:ss"
"+HHmm", -> "+HHmmss",
"+HH:mm", -> "+HH:mm:ss",
"+HHMM", -> "+HHmmss",
"+HH:MM", -> "+HH:mm:ss",
"+HHMMss", -> "+HHmmss",
"+HH:MM:ss", -> "+HH:mm:ss",
"+HHMMSS", -> "+HHmmss",
"+HH:MM:SS", -> "+HH:mm:ss",
"+HHmmss",
"+HH:mm:ss",
Note that the "+HH" pattern is a special case, as we don't know
whether to use the colon or non-colon pattern. Whether to require
colon or not is based on whether the next character after the HH is a
colon or not.
Proposed appendOffsetId() Javadoc:
* Appends the zone offset, such as '+01:00', to the formatter.
*
* This appends an instruction to format/parse the offset ID to the
builder.
* This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
* See {@link #appendOffset(String, String)} for details on formatting
and parsing.
Proposed appendOffset(String, String) Javadoc:
* During parsing, the offset...
changed to:
* When parsing in strict mode, the input must contain the mandatory
and optio
Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
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
Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
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
RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
Hi all, Please review an enhancement for a garbage free epochSecond method. Bug ID: https://bugs.openjdk.java.net/browse/JDK-8030864 <https://bugs.openjdk.java.net/browse/JDK-8030864> webrev: http://cr.openjdk.java.net/~ntv/8030864/webrev.01 <http://cr.openjdk.java.net/%7Entv/8030864/webrev.01> -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
Thanks and Regards,
Nadeesh
On 2/23/2016 5:17 PM, Stephen Colebourne wrote:
Thanks for the changes.
In `DateTimeFormatter`, the code should be
.parseLenient()
.appendOffsetId()
.parseStrict()
and the same in the other case. This ensures that existing callers who
then embed the formatter in another formatter (like the
ZONED_DATE_TIME constant) are unaffected.
The logic for lenient parsing does not look right as it only handles
types 5 and 6. This table shows the mappings needed:
"+HH", -> "+HHmmss" or "+HH:mm:ss"
"+HHmm", -> "+HHmmss",
"+HH:mm", -> "+HH:mm:ss",
"+HHMM", -> "+HHmmss",
"+HH:MM", -> "+HH:mm:ss",
"+HHMMss", -> "+HHmmss",
"+HH:MM:ss", -> "+HH:mm:ss",
"+HHMMSS", -> "+HHmmss",
"+HH:MM:SS", -> "+HH:mm:ss",
"+HHmmss",
"+HH:mm:ss",
Note that the "+HH" pattern is a special case, as we don't know
whether to use the colon or non-colon pattern. Whether to require
colon or not is based on whether the next character after the HH is a
colon or not.
Proposed appendOffsetId() Javadoc:
* Appends the zone offset, such as '+01:00', to the formatter.
*
* This appends an instruction to format/parse the offset ID to the builder.
* This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
* See {@link #appendOffset(String, String)} for details on formatting
and parsing.
Proposed appendOffset(String, String) Javadoc:
* During parsing, the offset...
changed to:
* When parsing in strict mode, the input must contain the mandatory
and optional elements are defined by the specified pattern.
* If the offset cannot be parsed then an exception is thrown unless
the section of the formatter is optional.
*
* When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional.
* The colons are required if the specified pattern contains a colon.
* If the specified pattern is "+HH", the presence of colons is
determined by whether the character after the hour digits is a colon
or not.
* If the offset cannot be parsed then an exception is thrown unless
the section of the formatter is optional.
thanks and sorry for delay
Stephen
On 11 February 2016 at 20:22, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please review a fix for
Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051
webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")
Gentle Reminder On 2/12/2016 1:52 AM, nadeesh tv wrote: Hi all, Please review a fix for Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051 webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/ -- Thanks and Regards, Nadeesh TV
RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")
Hi all, Please review a fix for Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051 webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/ -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations
Hi Shinya, Thnx. I will update it. Regards, Nadeesh On 2/3/2016 5:41 PM, ShinyaYoshida wrote: Hi Nadeesh, Almost LGTM!(But I'm not a reviewer;) ) However I've noticed that you don't use NANOS_PER_SECOND at L1223 and L1246. Is there some reason not to use it? Regards, shinyafox(Shinya Yoshida) 2016-02-01 15:18 GMT+09:00 nadeesh tv <nadeesh...@oracle.com <mailto:nadeesh...@oracle.com>>: Hi all, Please review following Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747 <https://bugs.openjdk.java.net/browse/JDK-8146747> Solution: Handled the negative duration separately webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/ <http://cr.openjdk.java.net/%7Entv/8146747/webrev.00/> <http://cr.openjdk.java.net/%7Entv/8146747/webrev.00/> -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations
Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8146747/webrev.01/ Regards, Nadeesh On 2/3/2016 5:48 PM, nadeesh tv wrote: Hi Shinya, Thnx. I will update it. Regards, Nadeesh On 2/3/2016 5:41 PM, ShinyaYoshida wrote: Hi Nadeesh, Almost LGTM!(But I'm not a reviewer;) ) However I've noticed that you don't use NANOS_PER_SECOND at L1223 and L1246. Is there some reason not to use it? Regards, shinyafox(Shinya Yoshida) 2016-02-01 15:18 GMT+09:00 nadeesh tv <nadeesh...@oracle.com <mailto:nadeesh...@oracle.com>>: Hi all, Please review following Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747 <https://bugs.openjdk.java.net/browse/JDK-8146747> Solution: Handled the negative duration separately webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/ <http://cr.openjdk.java.net/%7Entv/8146747/webrev.00/> <http://cr.openjdk.java.net/%7Entv/8146747/webrev.00/> -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations
Hi all, Please review following Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747 <https://bugs.openjdk.java.net/browse/JDK-8146747> Solution: Handled the negative duration separately webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/ <http://cr.openjdk.java.net/%7Entv/8146747/webrev.00/> -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit
Hi all, Thanks for the suggestions. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.01/ Regards, Nadeesh TV On 1/25/2016 10:24 PM, Roger Riggs wrote: Hi Stephen, Nadeesh, TimeUnit.toChronoUnit is a static method. It seems redundant to have to pass an instance to a static method of its type. cu = TimeUnit.toChronoUnit(TimeUnit.SECONDS); Instead of: TimeUnit tu = TimeUnit.SECONDS; ChronoUnit cu = tu.toChronoUnit(); Minor edits please: in @param and @return use the type name when referring to the type. For example, TimeUnit vs timeUnit (the parameter). in @throws, use the parameter name instead of "the unit"; For example, + * @throws IllegalArgumentException if timeUnit cannot be converted Thanks, Roger On 1/25/2016 11:06 AM, nadeesh tv wrote: Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV On 1/25/2016 9:01 PM, Stephen Colebourne wrote: Typo "TimeUnitequivalent" Otherwise looks good. thanks Stephen On 25 January 2016 at 15:25, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for conversion between Chronounit and Timeunit Bug ID : https://bugs.openjdk.java.net/browse/JDK-8141452 webrev: http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit
Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV On 1/25/2016 9:01 PM, Stephen Colebourne wrote: Typo "TimeUnitequivalent" Otherwise looks good. thanks Stephen On 25 January 2016 at 15:25, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for conversion between Chronounit and Timeunit Bug ID : https://bugs.openjdk.java.net/browse/JDK-8141452 webrev: http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit
Hi all, Please review a fix for conversion between Chronounit and Timeunit Bug ID : https://bugs.openjdk.java.net/browse/JDK-8141452 webrev: http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV
Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better
Hi Stephen,
It's already covered in
public void test_plusDays_invalidTooLarge() and public void
test_minusDays_maximum().
Thanks and Regards,
Nadeesh
On 1/9/2016 4:58 PM, Stephen Colebourne wrote:
Thanks for the update. You should have a test case for the exception
thrown by the checkValidValue()
Stephen
On 9 January 2016 at 09:33, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi Stephen/Roger,
Please see the updated the webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.03/
Explicit "YEAR.checkValidValue(year + 1);' added in 3rd case to handle the
invalidTooLarge year case (LocalDate.of(Year.MAX_VALUE, 12, 31).plusDays(1))
Thanks and Regards,
Nadeesh
On 1/9/2016 3:39 AM, Roger Riggs wrote:
+1
(With Stephen's update below).
Roger
On 1/8/2016 6:56 AM, Stephen Colebourne wrote:
As I mentioned in my email:
Rather than doing:
return withDayOfMonth((int) dom);
or
return LocalDate.of(year, month, (int) dom);
you can do
return new LocalDate(year, month, (int) dom);
(there are two occurrences)
Stephen
On 8 January 2016 at 10:56, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Thanks Stephen for the comments
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.02/
Regards,
Nadeesh
On 1/7/2016 6:15 PM, Stephen Colebourne wrote:
I updated the benchmark with this change and another one:
https://github.com/ThreeTen/threeten-bench/blob/master/src/main/java/org/threeten/LocalDateBenchmark.java
Marginally fastest is this pattern as it avoids one if statement:
if (dom > 0) {
if (dom <= 28) { // same month
return ...
}
if (dom <= 59) { // 59th Jan is 28th Feb
return ...
}
}
Rather than doing:
return LocalDate.of(year, month, (int) dom);
we can also do
return new LocalDate(year, month, (int) dom);
This is safe, because we know that the year, month and day are valid.
(I can't easily test the performance of this change, but it should be
noticeable as it avoids a lot of unnecessary checks).
I'd like a few more test cases. Addition around 27/28/29/30 from the
first of Jan/Feb, and of 13/14/15/16 from the 15th of Jan and 15th of
Feb.
Stephen
On 7 January 2016 at 09:20, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi ,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.01/
Thanks and regards,
Nadeesh
On 1/6/2016 12:11 AM, Roger Riggs wrote:
Hi Nadeesh,
LocalDate.java: +1374:
For the most common case of dom > 0 && <= 28, I would have explicitly
and
immediately returned the new LocalDate.
if (dom > 0 && dom <= 28) {
return LocalDate.of(year, month, (int) dom);
}
...
TCKLocalDate.java:
- Since the test_plusDays_normal is being replaced, its test case
should be
included in the DataProvider
{LocalDate.of(2007, 7, 15), 1, LocalDate.of(2007, 7, 16)}
Thanks, Roger
On 1/5/2016 1:05 PM, nadeesh tv wrote:
Hi all,
Please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8068803
web rev : http://cr.openjdk.java.net/~ntv/8068803/webrev.00/
Special thanks for Stephen for providing the source code patch
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better
Hi Stephen/Roger,
Please see the updated the webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.03/
Explicit "YEAR.checkValidValue(year + 1);' added in 3rd case to handle
the
*invalidTooLarge year case (*LocalDate.of(Year.MAX_VALUE, 12, 31).plusDays(1))
Thanks and Regards,
Nadeesh
On 1/9/2016 3:39 AM, Roger Riggs wrote:
+1
(With Stephen's update below).
Roger
On 1/8/2016 6:56 AM, Stephen Colebourne wrote:
As I mentioned in my email:
Rather than doing:
return withDayOfMonth((int) dom);
or
return LocalDate.of(year, month, (int) dom);
you can do
return new LocalDate(year, month, (int) dom);
(there are two occurrences)
Stephen
On 8 January 2016 at 10:56, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Thanks Stephen for the comments
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.02/
Regards,
Nadeesh
On 1/7/2016 6:15 PM, Stephen Colebourne wrote:
I updated the benchmark with this change and another one:
https://github.com/ThreeTen/threeten-bench/blob/master/src/main/java/org/threeten/LocalDateBenchmark.java
Marginally fastest is this pattern as it avoids one if statement:
if (dom > 0) {
if (dom <= 28) { // same month
return ...
}
if (dom <= 59) { // 59th Jan is 28th Feb
return ...
}
}
Rather than doing:
return LocalDate.of(year, month, (int) dom);
we can also do
return new LocalDate(year, month, (int) dom);
This is safe, because we know that the year, month and day are valid.
(I can't easily test the performance of this change, but it should be
noticeable as it avoids a lot of unnecessary checks).
I'd like a few more test cases. Addition around 27/28/29/30 from the
first of Jan/Feb, and of 13/14/15/16 from the 15th of Jan and 15th of
Feb.
Stephen
On 7 January 2016 at 09:20, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi ,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.01/
Thanks and regards,
Nadeesh
On 1/6/2016 12:11 AM, Roger Riggs wrote:
Hi Nadeesh,
LocalDate.java: +1374:
For the most common case of dom > 0 && <= 28, I would have
explicitly
and
immediately returned the new LocalDate.
if (dom > 0 && dom <= 28) {
return LocalDate.of(year, month, (int) dom);
}
...
TCKLocalDate.java:
- Since the test_plusDays_normal is being replaced, its test case
should be
included in the DataProvider
{LocalDate.of(2007, 7, 15), 1, LocalDate.of(2007, 7, 16)}
Thanks, Roger
On 1/5/2016 1:05 PM, nadeesh tv wrote:
Hi all,
Please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8068803
web rev : http://cr.openjdk.java.net/~ntv/8068803/webrev.00/
Special thanks for Stephen for providing the source code patch
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better
Hi all,
Thanks Stephen for the comments
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.02/
Regards,
Nadeesh
On 1/7/2016 6:15 PM, Stephen Colebourne wrote:
I updated the benchmark with this change and another one:
https://github.com/ThreeTen/threeten-bench/blob/master/src/main/java/org/threeten/LocalDateBenchmark.java
Marginally fastest is this pattern as it avoids one if statement:
if (dom > 0) {
if (dom <= 28) { // same month
return ...
}
if (dom <= 59) { // 59th Jan is 28th Feb
return ...
}
}
Rather than doing:
return LocalDate.of(year, month, (int) dom);
we can also do
return new LocalDate(year, month, (int) dom);
This is safe, because we know that the year, month and day are valid.
(I can't easily test the performance of this change, but it should be
noticeable as it avoids a lot of unnecessary checks).
I'd like a few more test cases. Addition around 27/28/29/30 from the
first of Jan/Feb, and of 13/14/15/16 from the 15th of Jan and 15th of
Feb.
Stephen
On 7 January 2016 at 09:20, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi ,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8068803/webrev.01/
Thanks and regards,
Nadeesh
On 1/6/2016 12:11 AM, Roger Riggs wrote:
Hi Nadeesh,
LocalDate.java: +1374:
For the most common case of dom > 0 && <= 28, I would have explicitly and
immediately returned the new LocalDate.
if (dom > 0 && dom <= 28) {
return LocalDate.of(year, month, (int) dom);
}
...
TCKLocalDate.java:
- Since the test_plusDays_normal is being replaced, its test case should be
included in the DataProvider
{LocalDate.of(2007, 7, 15), 1, LocalDate.of(2007, 7, 16)}
Thanks, Roger
On 1/5/2016 1:05 PM, nadeesh tv wrote:
Hi all,
Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8068803
web rev : http://cr.openjdk.java.net/~ntv/8068803/webrev.00/
Special thanks for Stephen for providing the source code patch
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
RFR:8146489:@since tag missed
Hi all, Please review a fix for BugID - https://bugs.openjdk.java.net/browse/JDK-8146489 Issue - while fixing JDK8142936 , I forgot to add @since 9 tag. webrev - http://cr.openjdk.java.net/~ntv/8146489/webrev.00/ -- Thanks and Regards, Nadeesh TV
RFR: JDK-8068803:Performance of LocalDate.plusDays could be better
Hi all, Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8068803 web rev : http://cr.openjdk.java.net/~ntv/8068803/webrev.00/ Special thanks for Stephen for providing the source code patch -- Thanks and Regards, Nadeesh TV
Re: RFR:8143413:add toEpochSecond methods for efficient access
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8143413/webrev.04/
Changes : Included the changes suggested by Stephen
Thanks and Regards,
Nadeesh
On 12/22/2015 12:30 AM, Stephen Colebourne wrote:
The comment for the new LocalDate.EPOCH field should use 1970-01-01,
not 1970-1-1. However, the code should omit the zeroes:
Replace:
LocalDate.of(1970, 01, 01)
with
LocalDate.of(1970, 1, 1)
The new method should follow the documentation of the similar method
on ChronoLocalDateTime:
* This combines this local date with the specified time and
offset to calculate the
* epoch-second value, which is the number of elapsed seconds from
1970-01-01T00:00:00Z.
* Instants on the time-line after the epoch are positive, earlier
are negative.
The same applies to the new method on LocalTime:
* This combines this local time with the specified date and
offset to calculate the
* epoch-second value, which is the number of elapsed seconds from
1970-01-01T00:00:00Z.
* Instants on the time-line after the epoch are positive, earlier
are negative.
The same applies to the new method on OffsetTime:
* This combines this offset time with the specified date to calculate the
* epoch-second value, which is the number of elapsed seconds from
1970-01-01T00:00:00Z.
* Instants on the time-line after the epoch are positive, earlier
are negative.
The test cases look good.
thanks
Stephen
On 17 December 2015 at 18:13, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8143413/webrev.03/
Thanks and Regards,
Nadeesh
On 12/4/2015 3:56 AM, Stephen Colebourne wrote:
In the interests of harmony and getting something in, I'll accept that.
I think LocalDate.EPOCH is probably better than LocalDate.EPOCHDAY
Stephen
On 3 December 2015 at 22:09, Roger Riggs<roger.ri...@oracle.com> wrote:
Hi Sherman,
It makes sense to me to provide the missing time/date as an explicit
parameter
for toEpochSeconds instead of assuming some constant.
localDate.toEpochSeconds(LocalTime.MIDNIGHT, ZoneOffset.UTC);
localTime.toEpochSeconds(LocalDate.EPOCHDAY, ZoneOffset.UTC);
offsetTime.toEpochSeconds(LocalDate.EPOCHDAY);
We'll have to add the LocalDate.EPOCHDAY constant.
It makes it a bit heavier weight to use but can still be optimized and
won't
create garbage.
Roger
On 12/01/2015 12:33 PM, Xueming Shen wrote:
On 12/1/15 6:36 AM, Stephen Colebourne wrote:
As Roger says, these new methods are about performance as well as
conversion.
While I fully acknowledge the time methods make an assumption, it is
not a crazy one, after all 1970-01-01 is just zero.
Key I think is it allows:
long epochSecs = date.toEpochSeconds(offset) +
time.toEpochSeconds(offset);
to efficiently merge two objects without garbage.
So it's not about j.t.LD/LT <-> j.u.Date, but instead of the clean
approach
LocalDate date = ...
LocalDate time = ...
ZoneOffset offset = ...
==> long spochSecs = LocalDateTime.of(date,
time).toEpochSeconds(offset);
we are adding APIs to provide a "fastpath" with the special assumption
that the LocalDate "date"
here is actually a "LocalDateTime" object ("date" + LocalTime.MIDNIGHT)
and the LocalTime "time"
again actually means a "LocalDateTime" (the "time" of 1970-01-01), to
let
the third party "libraries"
to fool the basic date/time abstract in java.time package, simply to
avoid creating the garbage
middle man, localDateTime? it really does not sound right to me.
First, if someone needs to mix/link LocalDate, LocalTime and Offset to
epoch seconds in their
libraries, they really SHOULD think hard about the conversion and make
it
right (it should not
be encouraged to use these classes LocalDate, LocalTime and Offset
without
even understand
what these classes are). But if we really have to provide such fastpath,
personally I think it might
be better either to provide these "utility/convenient" methods in a
"utilities" class, or with an
explicit date/time parameters (instead of the false assumption) for the
missing date/time piece,
such as
localDate.toEpochSeconds(LocalTime.MIDNIGHT, offset);
localTime.toEpochSeconds(LocalDate.EPOCHDAY, offset);
Sherman
It also means that no-one has to think hard about the conversion, as
it is just there. It tends to be when people try to work this stuff
out for themselves that they get it wrong.
Stephen
On 1 December 2015 at 14:21, Roger Riggs<roger.ri...@oracle.com>
wrote:
Hi Sherman,
On 11/30/2015 6:09 PM, Xueming Shen wrote:
On 11/30/2015 01:26 PM, Stephen Colebourne wrote:
Converting LocalDate<-> java.util.Date is the question with the
largest number of votes on SO:
http://stackoverflow.com/questions/21242110/convert-java-util-date-to-java-time-localdate/21242111
The proposed method is designed to make the convers
RFR:JDK-8145166 : Duration.toString violates specification
Hi all, Please review Bug Id - https://bugs.openjdk.java.net/browse/JDK-8145166 <https://bugs.openjdk.java.net/browse/JDK-8145166> Issue - Duration.toString violates specification for Durations which have value in the range -59 to -60, -119 to -120 seconds etc. webrev - http://cr.openjdk.java.net/~ntv/8145166/webrev.00/ <http://cr.openjdk.java.net/%7Entv/8145166/webrev.00/> -- Thanks and Regards, Nadeesh TV
Re: RFR:8143413:add toEpochSecond methods for efficient access
efault value", it is similar to the conversion from zdt->ldt->ld/lt, and I can see the "small" improvement from| Date input = new Date(); LocalDatedate =input.toInstant().atZone(ZoneId.systemDefault()).toLocalDate();| to |LocalDatedate =LocalDate.ofInstant(input.toInstant(),ZoneId.systemDefault()));| The proposed pair toEpochSecond() however is doing the other way around and with an unusual assumption of the missing date/time piece to a "default value" (midnight, the epoch day). personally I think localDate.atTime(LocalTime.MIDNIGHT).toEpochSecond(ZoneOffset); localTime.atDate(LocalDate.EPOCHDATE).toEpochSecond(ZoneOffset); is clean and good enough to help this backward conversion (with the addition of LocalDate.EPOCHDATE/DAY constant). Maybe, the vm can even help to remove that LocalDateTime middle man, with some arrangement. -Sherman Note that these methods are specifically not referencing java.util.Date itself. Epoch seconds is the appropriate intermediate form here, and still widely used. Stephen On 30 November 2015 at 19:36, Xueming Shen<xueming.s...@oracle.com> wrote: On 11/30/2015 10:37 AM, Stephen Colebourne wrote: This is based on user difficulties picked up via Stack Overflow. These methods aim to provide a simpler and faster approach, particularly for cases converting to/from java.util.Date. Can you be a little more specific on this one? We now have Instance<=> Date, and considerably we might add LocalDateTime<=> Date with an offset, if really really desired (for performance? to save a Instant object as the bridge?). But I'm a little confused about the connection among LocalDate/LocalTime, epoch seconds and j.u.Date here. Are you saying someone wants to convert j.t.LocalDate -> epoch seconds -> j.u.Date j.t.LocalTime -> epoch seconds -> j.u.Date and uses the converted j.u.Date to represent a local date (date with time part to be 0) and/or the local time (with year/month/day to be epoch time) in the "old" system which only has j.u.Date, and has to use the j.u.Date to abstract the "local date" and "local time"? I think we agreed back to JSR310 that we don't try to add such kind of "bridge/ convenient/utility" methods into the new java.time package, but only in the old date/calendar classes, if really needed. So if these methods are only to help migrate/bridge between the "old" and "new" calendar systems, the java.time might not be the best place for them? For the time cases, the convention of 1970-01-01 is natural and commonly used in many codebases. I'm not sure about that, especially the "natural" part. It might be "common" in the old days if you only have j.u.Date", and might be forced to use 1970-01-01 to fill in the "date" part even when you are really only interested in "time" part of it in your app. One of the advantage of java.time.LDT/LD/LT is now we have separate abstract for these different need, I don't see the common need of having a LocalTime only meas the "local time" of 1970-01-01, or I misunderstood something here. -Sherman Stephen On 30 November 2015 at 18:15, Xueming Shen<xueming.s...@oracle.com> wrote: Hi, While it is kinda understandable to have LocalDate.toEpochSecond(...) to get the epoch seconds since 1970.1.1, with the assumption of the time is at the midnight of that day. It is strange to have the Local/OffsetTime to have two public methods with the assumption of the "date" is at epoch year/month/day. What's the use case/scenario for these two proposed methods? -Sherman On 11/30/2015 06:36 AM, Stephen Colebourne wrote: The method Javadoc (specs) for each of the three new methods needs to be enhanced. For the date ones it needs to say "The resulting date will have a time component of midnight at the start of the day." For the time ones it needs to say "The resulting time will be on 1970-01-01." Some of the line wrapping in the tests looks like it is not indented correctly. Otherwise looks fine, thanks Stephen On 30 November 2015 at 11:50, nadeesh tv<nadeesh...@oracle.com> wrote: Hi all, Please review a fix for Bug Id -https://bugs.openjdk.java.net/browse/JDK-8143413 Issue - add toEpochSecond methods for efficient access webrev -http://cr.openjdk.java.net/~ntv/8143413/webrev.01/ -- Thanks and Regards, Nadeesh TV https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail; target="_blank">https://ipmcdn.avast.com/images/logo-avast-v1.png; style="width: 90px; height:33px;"/> This email has been sent from a virus-free computer protected by Avast.https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail; target="_blank" style="color: #4453ea;">www.avast.com -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8032510 : Add java.time.Duration.dividedBy(Duration)
HI all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032510/webrev.04/
Changes: chnaged the data provider as suggested
Regards,
Nadeesh
On 12/11/2015 9:24 PM, Roger Riggs wrote:
Hi Nadeesh,
The API looks fine.
I think the tests would be more readable if the Durations being tested
were created in the data provider.
Without a comment, it just looks like a lot of numbers.
The test methods arguments would then be (Duration dividend, Duration
divisor, long expected).
+ @DataProvider(name="dividedByDur_provider")
+ Object[][] provider_dividedByDur() {
+ return new Object[][] {
+ {new Duration.ofSeconds(0, 0), new Duration.ofSeconds(1, 0), 0},
etc.
Thanks, Roger
On 12/11/2015 7:14 AM, Stephen Colebourne wrote:
Fine by me.
Stephen
On 11 December 2015 at 11:53, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032510/webrev.03/
Regards,
Nadeesh TV
On 12/11/2015 4:45 PM, Stephen Colebourne wrote:
Missing blank line after the new method.
Typo: "diviosr"
Replace:
Objects.requireNonNull(divisor, "divisor is null");
with
Objects.requireNonNull(divisor, "divisor");
to match existing JSR-310 code.
Test case looks fine.
thanks
Stephen
On 11 December 2015 at 11:07, nadeesh tv <nadeesh...@oracle.com>
wrote:
Hi all,
Please review a fix for
Bug Id - https://bugs.openjdk.java.net/browse/JDK-8032510
Enhancement - Add java.time.Duration.dividedBy(Duration)
webrev - http://cr.openjdk.java.net/~ntv/8032510/webrev.02/
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR:JDK-8032510 : Add java.time.Duration.dividedBy(Duration)
Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8032510/webrev.03/ Regards, Nadeesh TV On 12/11/2015 4:45 PM, Stephen Colebourne wrote: Missing blank line after the new method. Typo: "diviosr" Replace: Objects.requireNonNull(divisor, "divisor is null"); with Objects.requireNonNull(divisor, "divisor"); to match existing JSR-310 code. Test case looks fine. thanks Stephen On 11 December 2015 at 11:07, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8032510 Enhancement - Add java.time.Duration.dividedBy(Duration) webrev - http://cr.openjdk.java.net/~ntv/8032510/webrev.02/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8032510 : Add java.time.Duration.dividedBy(Duration)
Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8032510 <https://bugs.openjdk.java.net/browse/JDK-8032510> Enhancement - Add java.time.Duration.dividedBy(Duration) webrev - http://cr.openjdk.java.net/~ntv/8032510/webrev.02/ <http://cr.openjdk.java.net/%7Entv/8032510/webrev.02/> -- Thanks and Regards, Nadeesh TV
Re: RFR: JDK-8142936:Additional java.time.Duration methods
Hi ,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8142936/webrev.04/
Thanks and Regards,
Nadeesh
On 12/4/2015 3:12 AM, Roger Riggs wrote:
Hi Nadeesh,
Thanks for the update, sorry I missed some editorial comments last time.
toSeconds method missing a final period '.'
"* This returns the total number of seconds in the duration"
toMinutesPart method: Extra text:
+ * @return the number of minutes parts in the duration, may be negative
+ * @since 9
*+ * may be negative*
+ */
+public int toMinutesPart(){
Missing space in multiple places: "(){" should have a space before "{"
toMillisPart and toNanosPart methods: unneeded description:
"+ * The total duration is defined by calling {@link #getNano()} and
{@link #getSeconds()}."
toMillisPart method: remove the final "."; it should be omitted on
@return, @param
"+ * @return the number of milliseconds part of the duration."
Thanks for coalescing the data providers.
Roger
On 12/03/2015 12:52 PM, nadeesh tv wrote:
Hi all,
Please see the updated webrev -
http://cr.openjdk.java.net/~ntv/8142936/webrev.03/
- changes - Modified the dataprovider as suggested by Roger
Thanks and regards,
Nadeesh
On 12/1/2015 2:39 AM, Stephen Colebourne wrote:
This is all about fixing a messy API that was created in 8. The
methods propose are all about consistency. The toSeconds() method
completes the set. For each unit there is a toXxx() and a toXxxPart().
Missing one out makes everything worse.
Stephen
On 30 November 2015 at 19:02, Roger Riggs <roger.ri...@oracle.com>
wrote:
Hi Stephen, Nadeesh,
The toXXXPart methods look fine.
I'm not entirely convinced that the toSeconds() method is worth it
and may
be
deemed as confusing with the new toSecondsPart method.
How many people have problems with getSeconds()?
The toXXXPart tests could have used a single DataProvider with
the values supplied for each unit to reduce the duplication.
Thanks, Roger
On 11/30/2015 09:29 AM, Stephen Colebourne wrote:
I think thats ready to be merged, thanks
Stephen
On 30 November 2015 at 11:26, nadeesh tv <nadeesh...@oracle.com>
wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8142936/webrev.02/
Regards,
Nadeesh TV
On 11/27/2015 11:20 PM, Stephen Colebourne wrote:
"Overall, looks pretty good.
There a a number of double spaces that should be single spaces
in the
Javadoc.
Change
"This is based on the standard definition of a day has 24 hours."
to
"This is based on the standard definition of a day as 24 hours."
("has" to "as")
There are three places to fix this.
The toMillisPart() docs could do with some rework.
change
"number of milliseconds part in the nanosecond part of the
duration"
to
"number of milliseconds part of the duration"
try this:
"This returns the milliseconds part by dividing the number of
nanoseconds by 1,000,000."
On the tests.
There is no test for toSeconds().
For the other tests, I have been bitten before by not testing edge
cases.
A test for zero, and for a negative round number would be good.
eg. for toHoursPart() the round number would be a duration of
exactly
minus 2 hours.
Duration.ofHours(2).toDaysPart() = 0
Duration.ofHours(2).toHoursPart() = 2
Duration.ofHours(2).toMinutesPart() = 0
Duration.ofHours(2).toSecondsPart() = 0
Duration.ofHours(2).toMillisPart() = 0
Duration.ofHours(2).toNanosPart() = 0
thus really six tests are needed each time. The best way to achieve
this is using @DataProvider in TestNG, where you can setup a
data grid
of inputs and 6 expected outputs.
thanks
Stephen
On 26 November 2015 at 06:41, nadeesh tv <nadeesh...@oracle.com>
wrote:
Hi all,
Please review a fix for
Bug Id - https://bugs.openjdk.java.net/browse/JDK-8142936
-Enhance Duration by adding toNanosPart() ,
toMillisPart(),toSecondsPart(),toMinutesPart(),toHoursPart(),toDaysPart()
methods .
- Had to rename privateBigDecimal toSeconds() ->
private
BigDecimal
toBigDecimalSeconds()
webrev - http://cr.openjdk.java.net/~ntv/8142936/webrev.01/
--
Thanks and Regards,
Nadeesh TV
href="https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail;
target="_blank">src="https://ipmcdn.avast.com/images/logo-avast-v1.png;
style="width:
90px; height:33px;"/>
#41424e;
font-size: 13px; font-family: Arial, Helvetica, sans-serif;
line-height: 18px;">This email has been sent from a virus-free
computer protected by Avast. href="https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail;
target="_blank" style="color: #4453ea;">www.avast.com
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
Re: RFR: JDK-8142936:Additional java.time.Duration methods
Hi all,
Please see the updated webrev -
http://cr.openjdk.java.net/~ntv/8142936/webrev.03/
- changes - Modified the dataprovider as suggested by Roger
Thanks and regards,
Nadeesh
On 12/1/2015 2:39 AM, Stephen Colebourne wrote:
This is all about fixing a messy API that was created in 8. The
methods propose are all about consistency. The toSeconds() method
completes the set. For each unit there is a toXxx() and a toXxxPart().
Missing one out makes everything worse.
Stephen
On 30 November 2015 at 19:02, Roger Riggs <roger.ri...@oracle.com> wrote:
Hi Stephen, Nadeesh,
The toXXXPart methods look fine.
I'm not entirely convinced that the toSeconds() method is worth it and may
be
deemed as confusing with the new toSecondsPart method.
How many people have problems with getSeconds()?
The toXXXPart tests could have used a single DataProvider with
the values supplied for each unit to reduce the duplication.
Thanks, Roger
On 11/30/2015 09:29 AM, Stephen Colebourne wrote:
I think thats ready to be merged, thanks
Stephen
On 30 November 2015 at 11:26, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8142936/webrev.02/
Regards,
Nadeesh TV
On 11/27/2015 11:20 PM, Stephen Colebourne wrote:
"Overall, looks pretty good.
There a a number of double spaces that should be single spaces in the
Javadoc.
Change
"This is based on the standard definition of a day has 24 hours."
to
"This is based on the standard definition of a day as 24 hours."
("has" to "as")
There are three places to fix this.
The toMillisPart() docs could do with some rework.
change
"number of milliseconds part in the nanosecond part of the duration"
to
"number of milliseconds part of the duration"
try this:
"This returns the milliseconds part by dividing the number of
nanoseconds by 1,000,000."
On the tests.
There is no test for toSeconds().
For the other tests, I have been bitten before by not testing edge
cases.
A test for zero, and for a negative round number would be good.
eg. for toHoursPart() the round number would be a duration of exactly
minus 2 hours.
Duration.ofHours(2).toDaysPart() = 0
Duration.ofHours(2).toHoursPart() = 2
Duration.ofHours(2).toMinutesPart() = 0
Duration.ofHours(2).toSecondsPart() = 0
Duration.ofHours(2).toMillisPart() = 0
Duration.ofHours(2).toNanosPart() = 0
thus really six tests are needed each time. The best way to achieve
this is using @DataProvider in TestNG, where you can setup a data grid
of inputs and 6 expected outputs.
thanks
Stephen
On 26 November 2015 at 06:41, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please review a fix for
Bug Id - https://bugs.openjdk.java.net/browse/JDK-8142936
-Enhance Duration by adding toNanosPart() ,
toMillisPart(),toSecondsPart(),toMinutesPart(),toHoursPart(),toDaysPart()
methods .
- Had to rename privateBigDecimal toSeconds() ->private
BigDecimal
toBigDecimalSeconds()
webrev - http://cr.openjdk.java.net/~ntv/8142936/webrev.01/
--
Thanks and Regards,
Nadeesh TV
https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail;
target="_blank">https://ipmcdn.avast.com/images/logo-avast-v1.png; style="width:
90px; height:33px;"/>
This email has been sent from a virus-free
computer protected by Avast. https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail;
target="_blank" style="color: #4453ea;">www.avast.com
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
RFR:JDK-8144349: @since tag missed
Hi all, Please review a fix for BugID - https://bugs.openjdk.java.net/browse/JDK-814434 Issue - while fixing JDK-8071919, JDK-8133079 I forgot to add @since 9 tag. webrev - http://cr.openjdk.java.net/~ntv/8144349/webrev.00/ -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8144349: @since tag missed
Hi , Sorry. I made a mistake. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8144349/webrev.01 Regards, Nadeesh On 12/1/2015 10:24 PM, Stephen Colebourne wrote: Those are not the right methods on LocalDate and LocalTime Stephen On 1 December 2015 at 16:50, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for BugID - https://bugs.openjdk.java.net/browse/JDK-814434 Issue - while fixing JDK-8071919, JDK-8133079 I forgot to add @since 9 tag. webrev - http://cr.openjdk.java.net/~ntv/8144349/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR: JDK-8142936:Additional java.time.Duration methods
Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8142936/webrev.02/
Regards,
Nadeesh TV
On 11/27/2015 11:20 PM, Stephen Colebourne wrote:
"Overall, looks pretty good.
There a a number of double spaces that should be single spaces in the Javadoc.
Change
"This is based on the standard definition of a day has 24 hours."
to
"This is based on the standard definition of a day as 24 hours."
("has" to "as")
There are three places to fix this.
The toMillisPart() docs could do with some rework.
change
"number of milliseconds part in the nanosecond part of the duration"
to
"number of milliseconds part of the duration"
try this:
"This returns the milliseconds part by dividing the number of
nanoseconds by 1,000,000."
On the tests.
There is no test for toSeconds().
For the other tests, I have been bitten before by not testing edge cases.
A test for zero, and for a negative round number would be good.
eg. for toHoursPart() the round number would be a duration of exactly
minus 2 hours.
Duration.ofHours(2).toDaysPart() = 0
Duration.ofHours(2).toHoursPart() = 2
Duration.ofHours(2).toMinutesPart() = 0
Duration.ofHours(2).toSecondsPart() = 0
Duration.ofHours(2).toMillisPart() = 0
Duration.ofHours(2).toNanosPart() = 0
thus really six tests are needed each time. The best way to achieve
this is using @DataProvider in TestNG, where you can setup a data grid
of inputs and 6 expected outputs.
thanks
Stephen
On 26 November 2015 at 06:41, nadeesh tv <nadeesh...@oracle.com> wrote:
Hi all,
Please review a fix for
Bug Id - https://bugs.openjdk.java.net/browse/JDK-8142936
-Enhance Duration by adding toNanosPart() ,
toMillisPart(),toSecondsPart(),toMinutesPart(),toHoursPart(),toDaysPart()
methods .
- Had to rename privateBigDecimal toSeconds() ->private BigDecimal
toBigDecimalSeconds()
webrev - http://cr.openjdk.java.net/~ntv/8142936/webrev.01/
--
Thanks and Regards,
Nadeesh TV
https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail;
target="_blank">https://ipmcdn.avast.com/images/logo-avast-v1.png; style="width:
90px; height:33px;"/>
This email has been sent from a virus-free
computer protected by Avast. https://www.avast.com/?utm_medium=email_source=link_campaign=sig-email_content=webmail;
target="_blank" style="color: #4453ea;">www.avast.com
--
Thanks and Regards,
Nadeesh TV
RFR:8143413:add toEpochSecond methods for efficient access
Hi all, Please review a fix for Bug Id -https://bugs.openjdk.java.net/browse/JDK-8143413 <https://bugs.openjdk.java.net/browse/JDK-8143413> Issue - add toEpochSecond methods for efficient access webrev - http://cr.openjdk.java.net/~ntv/8143413/webrev.01/ <http://cr.openjdk.java.net/%7Entv/8143413/webrev.01/> -- Thanks and Regards, Nadeesh TV
RFR: JDK-8142936:Additional java.time.Duration methods
Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8142936 <https://bugs.openjdk.java.net/browse/JDK-8142936> -Enhance Duration by adding toNanosPart() , toMillisPart(),toSecondsPart(),toMinutesPart(),toHoursPart(),toDaysPart() methods . - Had to rename privateBigDecimal toSeconds() ->private BigDecimal toBigDecimalSeconds() webrev - http://cr.openjdk.java.net/~ntv/8142936/webrev.01/ <http://cr.openjdk.java.net/%7Entv/8142936/webrev.01/> -- Thanks and Regards, Nadeesh TV
RFR:JDK-8071919 :Add java.time.Clock.tickMillis(ZoneId zone) method
Hi all, Please review a fix for Bug Id -https://bugs.openjdk.java.net/browse/JDK-8071919 <https://bugs.openjdk.java.net/browse/JDK-8071919> Issue - Add java.time.Clock.tickMillis(ZoneId zone) method webrev - http://cr.openjdk.java.net/~ntv/8071919/webrev.01/ <http://cr.openjdk.java.net/%7Entv/8071919/webrev.01/> -- Thanks and Regards, Nadeesh TV
RFR:JDK-8072746:LocalDate.isEra() should return IsoEra not Era
Hi all, Please review a fix for BugId - https://bugs.openjdk.java.net/browse/JDK-8072746 <https://bugs.openjdk.java.net/browse/JDK-8072746> Issue - LocalDate.isEra() should return IsoEra not Era webrev - http://cr.openjdk.java.net/~ntv/8072746/webrev.02 <http://cr.openjdk.java.net/%7Entv/8072746/webrev.01> -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8071919 :Add java.time.Clock.tickMillis(ZoneId zone) method
Hi , Please see the updated webrev http://cr.openjdk.java.net/~ntv/8071919/webrev.02/ Thanks and Regards, Nadeesh On 11/13/2015 9:23 PM, Roger Riggs wrote: Hi Nadeesh, One suggestion: Replace "This clock will always have the other than milli part of nano-of-second field set to zero." Though similar to the other methods, the "other than milli part" is awkward. With: "This clock will always have the nano-of-second field truncated to milliseconds" The rest looks fine. Thanks, Roger On 11/13/2015 6:00 AM, nadeesh tv wrote: Hi all, Please review a fix for Bug Id -https://bugs.openjdk.java.net/browse/JDK-8071919 <https://bugs.openjdk.java.net/browse/JDK-8071919> Issue - Add java.time.Clock.tickMillis(ZoneId zone) method webrev - http://cr.openjdk.java.net/~ntv/8071919/webrev.01/ <http://cr.openjdk.java.net/%7Entv/8071919/webrev.01/> -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8072746:LocalDate.isEra() should return IsoEra not Era
Hi all,
Please review the updated webrev
http://cr.openjdk.java.net/~ntv/8072746/webrev.03/
Thanks and Regards,
Nadeesh TV
On 11/13/2015 8:40 PM, Roger Riggs wrote:
Hi Nadeesh,
The @return mentions "ISOChronoloy era constant" and it should
probably be a reference to IsoEra.
Also make it a link {@link java.time.chrono.IsoEra IsoEra}
Otherwise looks good.
Thanks, Roger
On 11/13/2015 6:06 AM, nadeesh tv wrote:
Hi all,
Please review a fix for BugId -
https://bugs.openjdk.java.net/browse/JDK-8072746
<https://bugs.openjdk.java.net/browse/JDK-8072746>
Issue - LocalDate.isEra() should return IsoEra not Era
webrev - http://cr.openjdk.java.net/~ntv/8072746/webrev.02
<http://cr.openjdk.java.net/%7Entv/8072746/webrev.01>
--
Thanks and Regards,
Nadeesh TV
--
Thanks and Regards,
Nadeesh TV
RFR:JDK-8054978:java.time.Duration.parse() fails for negative duration with 0 seconds and nanos
Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8054978 <https://bugs.openjdk.java.net/browse/JDK-8054978> Issue - While parsing a negative Duration with 0 seconds and a few nanoseconds, the nanoseconds are wrongly considered positive Apart from the above , corrected a documentation error in the examples for Duration.parse(CharSequence) webrev - http://cr.openjdk.java.net/~ntv/8054978/webrev.02 <http://cr.openjdk.java.net/%7Entv/8054978/webrev.01/> -- Thanks and Regards, Nadeesh TV
Re: RFR:JDK-8133079:java.time LocalDate and LocalTime ofInstant() factory methods
Hi Stephen, Yes, I will send out a separate RFR for JDK-8030864 Regards, Nadeesh On 11/12/2015 7:11 PM, Stephen Colebourne wrote: Looks good to me. Need to ensure that JDK-8030864 also gets tackled, as it is the inverse operation of these new methods. thanks Stephen On 12 November 2015 at 06:40, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8133079 Enhancement - Enhance LocalDate and LocalTime by adding .ofInstant(Instant, ZoneId) webrev - http://cr.openjdk.java.net/~ntv/8133079/webrev.01/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:JDK-8133079:java.time LocalDate and LocalTime ofInstant() factory methods
Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8133079 Enhancement - Enhance LocalDate and LocalTime by adding .ofInstant(Instant, ZoneId) webrev - http://cr.openjdk.java.net/~ntv/8133079/webrev.01/ -- Thanks and Regards, Nadeesh TV
RFR:JDK-8138664- ZonedDateTime parse error for any date using 'GMT0' ZoneID
Hi all, Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8138664 Issue- ZonedDateTime parse error for any date using 'GMT0' ZoneID Solution - Handled the GMT0 separately in the parsing method BugId - https://bugs.openjdk.java.net/browse/JDK-8138664 webrev - http://cr.openjdk.java.net/~ntv/8138664/webrev.01 -- Thanks and Regards, Nadeesh TV
RFR:JDK-8066571:UnsupportedTemporalTypeException is thrown not only in the case of unsupported temporal
Hi all, Please review a fix for Bug Id - https://bugs.openjdk.java.net/browse/JDK-8066571 <https://bugs.openjdk.java.net/browse/JDK-8066571> Issue - IsoFields.Unit.issupportedBy was not checking isIso() Solution - Moved isIso() from Filed to IsoFields sothat both Field and Unit can use Apart from the above fix, corrected a documentaion error in IsoFields - QUARTER_OF_YEAR webrev - http://cr.openjdk.java.net/~ntv/8066571/webrev.04/ <http://cr.openjdk.java.net/%7Entv/8066571/webrev.04/> -- Thanks and Regards, Nadeesh TV
RFR:JDK-8138664- ZonedDateTime parse error for any date using 'GMT0' ZoneID
Hi all, Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8138664 Issue- ZonedDateTime parse error for any date using 'GMT0' ZoneID Solution - Handled the GMT0 separately in the parsing method BugId - https://bugs.openjdk.java.net/browse/JDK-8138664 webrev - http://cr.openjdk.java.net/~ntv/8138664/webrev.00/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
Re: RFR:8134928:java.time.Instant.truncatedTo(TemporalUnit unit) is truncating up if the year < 1970
HI all, Please review the updated webrev http://cr.openjdk.java.net/~ntv/8134928/webrev.01/ Regards, Nadeesh On 10/14/2015 4:06 PM, Stephen Colebourne wrote: I'd like to see an additional test case for the cases where rounding is *not* needed. Currently, there are only tests for where rounding is needed. It needs one more test line for after 1970 and one for before 1970. thanks Stephen On 14 October 2015 at 10:53, nadeesh tv <nadeesh...@oracle.com> wrote: Hi all, Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8134928 Issue- java.time.Instant.truncatedTo(TemporalUnit unit) is truncating up if the year < 1970 BugId - https://bugs.openjdk.java.net/browse/JDK-8134928 webrev - http://cr.openjdk.java.net/~pchopra/8134928/webrev.01/ -- Thanks and Regards, Nadeesh TV -- Thanks and Regards, Nadeesh TV
RFR:8134928:java.time.Instant.truncatedTo(TemporalUnit unit) is truncating up if the year < 1970
Hi all, Please review a fix for https://bugs.openjdk.java.net/browse/JDK-8134928 Issue- java.time.Instant.truncatedTo(TemporalUnit unit) is truncating up if the year < 1970 BugId - https://bugs.openjdk.java.net/browse/JDK-8134928 webrev - http://cr.openjdk.java.net/~pchopra/8134928/webrev.01/ -- Thanks and Regards, Nadeesh TV
JDK-8074023: Clock.system(ZoneId) could be optimized to always return the same clock for a given zone
Hi all, Please review this minor change which optimized Clock.systemDefaultZone() and Clock.system(ZoneId) to avoid creating new instance of Clock.SystemClock every time they are called. Bug ID : https://bugs.openjdk.java.net/browse/JDK-8074023 Webrev : http://cr.openjdk.java.net/~rriggs/nadeesh-tv-8074023/ -- Thanks and Regards, Nadeesh TV Oracle IDC, 6th Floor Divyasree Chambers Mob: 9986800452
Code review request for JDK-8076441: Remove dead code in java.time.chrono.Chronology.isLeapYear
Hi all, Please review this minor change which remove a dead code in isLeapYear(long prolepticYear) method of java/time/chrono/HijrahChronology.java. Bug: https://bugs.openjdk.java.net/browse/JDK-8076441 Webrev: http://cr.openjdk.java.net/~rriggs/hadeesh-tv-8076441/ -- Thanks and Regards, Nadeesh TV
