[15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

[Joe Wang]

Thanks Roger, Naoto.

On 6/9/2020 12:53 PM, Roger Riggs wrote:
> Hi,
>
> All java.time instances are immutable and described as value based.
> Identity based operations are defined as unpredictable and to be avoided.
> (That specific text was omitted from DateTimeFormatter)
>
> The test uses .equals so it is not depending on identity.
>
> Roger
>
>
> On 6/9/20 3:10 PM, naoto.sato at oracle.com wrote:
>> Hi Joe,
>>
>> On 6/9/20 11:54 AM, Joe Wang wrote:
>>> Looks fine to me for the implementation of the method.
>>
>> Thank you for the review!
>>
>>>
>>> It's interesting to observe that the withXXXX methods, as 
>>> demonstrated in the test, always create a new DateTimeFormatter 
>>> instance. The Javadoc also said so, e.g. "Returns a copy of this 
>>> formatter with a new XXXX", and by "a copy" it meant a new instance. 
>>> Now the localizedBy method also "Returns a copy of this formatter", 
>>> but it may actually returns this formatter. Would it confuse users 
>>> (into thinking they always get a new instance)?  Would we need to 
>>> amend the javadoc a bit, e.g. indicating it returns this formatter 
>>> if all of the above aspects are the same? (or otherwise a new 
>>> instance is returned?)
>>
>> Yes we would, but it's not specific to localizedBy() method but 
>> common to all methods. So I would rather follow the same pattern with 
>> other methods for this changeset.
>>
>>>
>>> The localizedBy method seems to me it's not "Unlike the withLocale 
>>> method" in terms of "producing a different formatter". The 
>>> difference lies in the attributes demonstrated in this changeset, 
>>> e.g. zone, chrono and decimalStyle will be from the specified locale 
>>> in localizedBy, while with the withLocale method, they are 
>>> inherited. In other words, the withXXXX methods only changes the 
>>> specified field.
>>
>> That's exactly why this localzedBy() was introduced. withLocale() 
>> only replaces the 'locale' field in the formatter, and no other 
>> fields/format patterns, even though they are locale related, are 
>> modified. "Unlike ..." means to describe it.
>>
>> Naoto
>>
>>>
>>> -Joe
>>>
>>>>
>>>> Naoto
>>>>
>>>>>
>>>>> Thanks, Roger
>>>>>
>>>>>
>>>>>
>>>>> On 6/8/20 5:06 PM, naoto.sato at oracle.com wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please review the fix to the following issue:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8246662
>>>>>>
>>>>>> The proposed changeset is located at:
>>>>>>
>>>>>> https://cr.openjdk.java.net/~naoto/8246662/webrev.00/
>>>>>>
>>>>>> This is a regression caused by the fix to JDK-8244245, where the 
>>>>>> locale related fields in the formatter are now overridden by 
>>>>>> localizedBy() method. It was only comparing the locale object for 
>>>>>> the fast path, but it was not sufficient.
>>>>>>
>>>>>> Naoto
>>>>>
>>>
>