On Wed, 21 May 2025 21:19:36 GMT, Justin Lu <j...@openjdk.org> wrote:
> _sun.util.Locale.LanguageTag_ is essentially a BCP47 language tag data > carrier for Locale. The class, once created is not modified; the class should > be made immutable. Converting the class to a record accomplishes this and > also simplifies some of the existing code. Nice refactoring. Couple of nits follow src/java.base/share/classes/sun/util/locale/LanguageTag.java line 53: > 51: public static final String PRIVUSE_VARIANT_PREFIX = "lvariant"; > 52: private static final String EMPTY_TAG = ""; > 53: private static final List<String> EMPTY_TAGS = > Collections.emptyList(); It would be clearer to rename "TAG(S)" to "SUBTAG(S)" src/java.base/share/classes/sun/util/locale/LanguageTag.java line 191: > 189: String region = EMPTY_TAG; > 190: List<String> variants = EMPTY_TAGS; > 191: List<String> extensions = EMPTY_TAGS; These assignments can be moved for the case of `language.isEmpty()` ------------- PR Review: https://git.openjdk.org/jdk/pull/25371#pullrequestreview-2859485182 PR Review Comment: https://git.openjdk.org/jdk/pull/25371#discussion_r2101431166 PR Review Comment: https://git.openjdk.org/jdk/pull/25371#discussion_r2101426074
