On Tue, 19 Nov 2024 07:58:22 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review this PR and corresponding CSR which includes a wide range of >> specification improvements for java.util.Locale. See the CSR for further >> detail. Other changes/suggestions are welcomed to be included as part of >> this change. >> >> APIDiff: >> https://cr.openjdk.org/~jlu/8341923_apidiff/java.base/java/util/Locale.html >> Javadoc: https://cr.openjdk.org/~jlu/api/java.base/java/util/Locale.html > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > address Chen's review; make other similar changes Looks good. Some comments that I missed in the initial (internal) review. src/java.base/share/classes/java/util/Locale.java line 89: > 87: * exchange. Each {@code Locale} is associated with locale data which is > retrieved > 88: * by the installed {@link java.util.spi.LocaleServiceProvider > LocaleServiceProvider} > 89: * implementations. Depending on the implementation, such data may vary > by release. This reads like locale data are only obtained with the SPI implementations that are provided by the users. The locale data also (and mainly) come from within Java runtime itself. (BTW, "installed" should be removed, as it is no longer correct. Same applies to getAvailableLocales()/availableLocales() methods) src/java.base/share/classes/java/util/Locale.java line 98: > 96: * Locale.forLanguageTag("en-Latn-US-POSIX-u-nu-latn")} would be > comprised of all > 97: * the fields below. This particular {@code Locale} would represent > English in > 98: * the United States using the Latin alphabet and numerics for use in > POSIX "script" is more precise than "alphabet", as the latter suggests only letters [A-Z] src/java.base/share/classes/java/util/Locale.java line 144: > 142: * each indicating its own semantics, these values should be ordered > 143: * by importance, with most important first, separated by > 144: * underscore('_'). The variant field is case sensitive.</dd> This part "separated by underscore('_')" is missing in the revised doc src/java.base/share/classes/java/util/Locale.java line 216: > 214: * overloads do not make any syntactic checks on the input. > 215: * > 216: * <h3><a id="def_locale_extension">Unicode locale/language > extension</a></h3> Probably using the exact name in the external spec is better [Unicode BCP 47 U Extension](https://unicode.org/reports/tr35/#u_Extension) src/java.base/share/classes/java/util/Locale.java line 218: > 216: * <h3><a id="def_locale_extension">Unicode locale/language > extension</a></h3> > 217: * > 218: * <p>UTS#35, "Unicode Locale Data Markup Language" defines optional UTS35 (https://unicode.org/reports/tr35/) should be listed with @spec link src/java.base/share/classes/java/util/Locale.java line 344: > 342: * to BCP 47 syntax. Use a builder to enforce syntactic restrictions on > the input.</dd> > 343: * </dl> > 344: * <p>The following are all equivalent: "all equivalent" seems a bit vague, as the actual implementation is not equivalent (unlike other examples below). Something like "the following all produce equivalent locale objects" or along the lines? ------------- PR Review: https://git.openjdk.org/jdk/pull/22192#pullrequestreview-2446512394 PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1848969909 PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849007375 PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849018322 PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849026657 PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849027660 PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849032564
