[13] RFR: CONFIG level logging statements printed in CLDRCalendarDataProviderImpl.java even when default log Level is INFO

Roger Riggs roger.riggs at oracle.com
Thu Feb 21 02:41:32 UTC 2019


Hi Naoto,

The fix looks fine.

The direction for new tests is to give them functional names, not bugids.
Is there a suitable name?

CalendarDataUtility.java: 260;  the assert is just documentation right?
Its rare to see asserts enabled except in test contexts.

Thanks, Roger


On 2/20/19 5:54 PM, naoto.sato at 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
>>>>
>>



More information about the core-libs-dev mailing list