<i18n dev> RFR: 8321480: ISO 4217 Amendment 176 Update [v2]

Justin Lu jlu at openjdk.org
Mon Dec 11 03:03:45 UTC 2023


On Thu, 7 Dec 2023 22:20:29 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8321480-ISO4217-176
>>  - add comment
>>  - reflect review comments
>>  - add xcg to CurrencyNames
>>  - init
>
> src/java.base/share/classes/sun/util/resources/CurrencyNames.properties line 497:
> 
>> 495: xbd=European Unit of Account (XBD)
>> 496: xcd=East Caribbean Dollar
>> 497: xcg=Caribbean Guilder
> 
> I think `XCG=XCG` is also needed for not throwing `MissingResourceException` for `getSymbol()`

Thanks for the correction, added in.

> test/jdk/java/util/Currency/ValidateISO4217.java line 181:
> 
>> 179:                     // without updating ISO4217Codes
>> 180:                     String futureCurr = tokens.nextToken();
>> 181:                     testCurrencies.add(Currency.getInstance(futureCurr));
> 
> I'd not add the future currency, and fix it in the code not to add future currency in available currencies.

Updated the Currency build process to disallow any Currencies that are future Currencies. This prevents the future currency, "XCG" from leaking out into `Currency.getAvailableCurrencies()`. (This was only exposed now, since the previous future Currencies were already ones expected to be found in `Currency.getAvailableCurrencies()`. E.g. Amendment 174 where HRK was replaced by EUR.

> test/jdk/java/util/Currency/ValidateISO4217.java line 289:
> 
>> 287:                 assertNull(Currency.getInstance(Locale.of("", country)),
>> 288:                         "Error: Currency.getInstance() for this locale should return null: " + country);
>> 289:             }
> 
> What is this change for?

The previous output was overflowing due to the method being a `@ParameterizedTest` since it was testing against 26*26 inputs. Changing to `@Test` makes diagnostics easier, as there is no more overflow.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1421898797
PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1421898780
PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1421898803


More information about the i18n-dev mailing list