Thanks for the updates, Looks good.
On 02/21/2019 12:06 PM, Naoto Sato wrote:
Hi Roger,
Sorry for the missing the bug id in the subject, added it. Better late
than never :-)
On 2/20/19 6:41 PM, Roger Riggs wrote:
Hi Naoto,
The fix looks fine.
The direction for new tests is to give them functional names, not
bugids.
Is there a suitable name?
Renamed the test case, and modified @summary text accordingly.
CalendarDataUtility.java: 260; the assert is just documentation right?
Its rare to see asserts enabled except in test contexts.
Yes, it's meant for jtreg environment so that they are logged (if ever).
Updated webrev: http://cr.openjdk.java.net/~naoto/8218960/webrev.01/
Naoto
Thanks, Roger
On 2/20/19 5:54 PM, naoto.s...@oracle.com wrote:
Thanks, Nishit.
I'd still like to ask for a review from a Reviewer.
Naoto
On 2/20/19 12:33 AM, Nishit Jain wrote:
Hi Naoto,
Thanks for the explanation. Change looks fine to me.
Regards,
Nishit Jain
On 19-02-2019 22:51, Naoto Sato wrote:
Hi Nishit,
The reason is that "US" is the only required locale in the JDK
(cf. Locale.getAvailableLocales(). In fact, initially I supplied
"001" with it, as it means the "world" in CLDR, but it broke some
existing tests. "001" returns "MONDAY" as the first day of week,
whereas "SUNDAY" in US. For the compatibility reason, I had to
resort to "US". I am not sure we want to hardcode "1" in this case
without any convincing reason.
Naoto
On 2/19/19 6:37 AM, Nishit Jain wrote:
Hi Naoto,
Why is the default region set to "US" if there is no region
specified in the locale? is this the default behavior of "first
day of week" and "minimal days in first week" when a region is
missing or the default behavior is that it returns "1"? Can't we
just return "1" instead of setting the region to "US"?
Regards,
Nishit Jain
On 16-02-2019 04:25, Naoto Sato wrote:
Hello,
Please review the fix to the following issue:
https://bugs.openjdk.java.net/browse/JDK-8218960
The proposed changeset is located at:
http://cr.openjdk.java.net/~naoto/8218960/webrev.00/
The CONFIG message was generated because
CLDRCalendarDataProviderImpl was returning null for locales
without region. Use "US" as the default region in such a case.
Naoto
