Re: RFR: 8174269: Remove COMPAT locale data provider from JDK [v5]
On Tue, 27 Feb 2024 11:24:08 GMT, Magnus Ihse Bursie wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressing review comments > > This looks good from a build perspective. Actually, it looks great! :) I'm > happy to get rid of this old strange construct. Thanks, @magicus > to get rid of this old strange construct. Removing legacy clutter is exactly the purpose of this exercise! - PR Comment: https://git.openjdk.org/jdk/pull/17991#issuecomment-1967125233
Re: RFR: 8174269: Remove COMPAT locale data provider from JDK [v5]
On Tue, 27 Feb 2024 00:24:08 GMT, Naoto Sato wrote: >> This PR intends to remove the legacy `COMPAT` locale data from the JDK. The >> `COMPAT` locale data was introduced for applications' migratory purposes >> transitioning to `CLDR`. It is becoming a technical debt and now is the time >> to remove it (we've been emitting a warning at JVM startup since JDK21, if >> the app is using `COMPAT`). A corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Addressing review comments This looks good from a build perspective. Actually, it looks great! :) I'm happy to get rid of this old strange construct. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17991#pullrequestreview-1903238470
Re: RFR: 8174269: Remove COMPAT locale data provider from JDK [v5]
On Mon, 26 Feb 2024 22:11:31 GMT, Justin Lu wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Addressing review comments
>
> src/java.base/share/classes/sun/util/locale/provider/FallbackLocaleProviderAdapter.java
> line 86:
>
>> 84: @Override
>> 85: // In order to correctly report supported locales
>> 86: public BreakIteratorProvider getBreakIteratorProvider() {
>
> More for my understanding but I am curious why FallbackLocaleProviderAdapter
> has to override `getBreakIteratorProvider`, but can rely on the
> `getCollatorProvider` from JRELocaleProviderAdapter? Also wondering why
> "BreakIteratorRules" is fetched when JRELocaleProviderAdapter fetches
> "FormatData" if the data is the same COMPAT data.
`COMPAT` used to offer supported locales for the ones that exist as resource
bundles. For `Collator`, `JRELocaleProviderAdapter` had a list for
`CollationData` resource bundles, but `BreakIterator` shared with `FormatData`,
which now only has root/en/ja (for Gan-nen support). So it had to override the
method and return `th` (this is the main function for `BreakIterator` as of now)
> test/jdk/java/text/Format/NumberFormat/CurrencyFormat.java line 30:
>
>> 28: * Tests both COMPAT and CLDR data.
>> 29: * @modules jdk.localedata
>> 30: * @run junit/othervm -Djava.locale.providers=COMPAT CurrencyFormat
>
> The methods `currencySymbolsTest`, `currencySymbolsDataProvider`, and
> `getFutureSymbol` can be removed since they are for COMPAT only.
>
> The string array `expectedCOMPATData` can be removed from the data provider
> method `currencyFormatDataProvider` as well as `isCompat` variable and usage.
>
> _CurrencySymbols.properties_ can also be deleted since that is what
> `currencySymbolsDataProvider` uses to build the data and no other tests rely
> on the file.
Good catch! Removed `COMPAT` related tests/data.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1503478694
PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1503478605
Re: RFR: 8174269: Remove COMPAT locale data provider from JDK [v5]
> This PR intends to remove the legacy `COMPAT` locale data from the JDK. The > `COMPAT` locale data was introduced for applications' migratory purposes > transitioning to `CLDR`. It is becoming a technical debt and now is the time > to remove it (we've been emitting a warning at JVM startup since JDK21, if > the app is using `COMPAT`). A corresponding CSR has also been drafted. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Addressing review comments - Changes: - all: https://git.openjdk.org/jdk/pull/17991/files - new: https://git.openjdk.org/jdk/pull/17991/files/a75637f2..d5953788 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17991&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17991&range=03-04 Stats: 232 lines in 4 files changed: 4 ins; 226 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17991.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17991/head:pull/17991 PR: https://git.openjdk.org/jdk/pull/17991
