On Mon, 11 Dec 2023 03:03:44 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review this PR which incorporates the ISO 4217 Amendment 176 Update. >> As the replacement of `ANG` to `XCG` won't occur until 2025, this change >> does not need to go into JDK22. `HR` was also updated to remove the past >> cutover dates. >> >> An existing test in _ValidateISO4217.java_ checked that >> `Currency::getAvailableCurrencies` had all the expected currencies. This >> method returns all currencies, including ones to take place in the future >> (e.g. `XCG`). The expected currencies `Set` the method was test against had >> to be updated to also include future currencies as well. >> >> Additionally, this change also converted a parameterized test to a normal >> JUnit test, due to output overflow. > > 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 make/jdk/src/classes/build/tools/generatecurrencydata/GenerateCurrencyData.java line 347: > 345: // otherwise it will leak out into > Currency:getAvailableCurrencies > 346: boolean notFutureCurrency = > !Arrays.asList(specialCaseNewCurrencies).contains(currencyCode); > 347: boolean notSimpleCurrency = tableEntry == > INVALID_COUNTRY_ENTRY I'd rather not bundle up those conditions into `notSimpleCurrency`, as it obscures the other two cases. Simply OR-ing those conditions and `notFutureCurrency` ( or `!futureCurrency` may be more readable) would be sufficient IMO. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17023#discussion_r1423036797
