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