<i18n dev> [14] RFR: 8230284: Accounting currency format support does not cope with explicit number system.

naoto.sato at oracle.com naoto.sato at oracle.com
Mon Sep 9 18:23:12 UTC 2019


Hi Roger,

Thanks for the review.

On 9/9/19 8:12 AM, Roger Riggs wrote:
> Hi Naoto,
> 
> Bundle.java:
>   - 28: Explicit import are preferred,  (if the IDE make the change, fix 
> the settings in the IDE).

Corrected.

> 
>   - 763, the indentation of the nested getOrDefault calls doesn't look 
> conventional.
>    (It might be interesting to have an API that allows one or more 
> defaults to be tried in turn.
>     It will call each of the methods so there is no shortcutting if the 
> value is found early in the list).

The code meant to short cut if a value is found early, as this 
implements the fallback. I corrected the indentation of getOrDefault() 
methods.

> 
> CLDRConverter:
>   - 513, 1169...: Debugging code can be removed

I removed the one around line 513, but kept the 1169 as this is useful. 
It does not affect the JDK image as this is the code for the build tool.

> 
> Otherwise looks ok.

Updated webrev is here:

http://cr.openjdk.java.net/~naoto/8230284/webrev.01/

Naoto

> 
> Thanks, Roger
> 
> 
> 
> On 9/6/19 1:59 PM, naoto.sato at oracle.com wrote:
>> Hello,
>>
>> Please review the fix to the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8230284
>>
>> The proposed changeset is located at:
>>
>> https://cr.openjdk.java.net/~naoto/8230284/webrev.00/
>>
>> The original enhancement for the accounting currency format support 
>> (https://bugs.openjdk.java.net/browse/JDK-8215181) did not account for 
>> the explicit/implicit numbering script. The above change made it to 
>> share the same code with NumberElements which properly deals with the 
>> numbering script.
>>
>> Naoto
> 


More information about the i18n-dev mailing list