On Thu, 7 Dec 2023 22:20:29 GMT, Naoto Sato <na...@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
