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

[Roger Riggs]

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
>>>>
>>