Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
On Wed, 10 Nov 2021 19:05:17 GMT, Naoto Sato wrote: >> This fix is to require to include `Locale.ROOT` in the returned arrays/set >> from `getAvailableLocales()` methods in various locale-sensitive classes. >> The implementation has been including `Locale.ROOT` since its inception, it >> is simply a doc clarification (+ a test case verifying it). Corresponding >> CSR has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Using @code tag for `Set`. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
On Wed, 10 Nov 2021 19:05:17 GMT, Naoto Sato wrote: >> This fix is to require to include `Locale.ROOT` in the returned arrays/set >> from `getAvailableLocales()` methods in various locale-sensitive classes. >> The implementation has been including `Locale.ROOT` since its inception, it >> is simply a doc clarification (+ a test case verifying it). Corresponding >> CSR has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Using @code tag for `Set`. Nice use of MethodHandles.lookup() in the test. - Marked as reviewed by smarks (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
On Wed, 10 Nov 2021 18:29:06 GMT, Pavel Rappo wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Using @code tag for `Set`.
>
> src/java.base/share/classes/java/time/format/DecimalStyle.java line 118:
>
>> 116: * Lists all the locales that are supported.
>> 117: *
>> 118: * At a minimum, the returned Set must contain a {@code Locale}
>> instance equal to
>
> A nit, really. Consider applying either of these suggestions:
> Suggestion:
>
> * At a minimum, the returned {@code Set} must contain a {@code Locale}
> instance equal to
>
> Suggestion:
>
> * At a minimum, the returned set must contain a {@code Locale} instance
> equal to
Thanks. Adopted the former suggestion.
-
PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT [v2]
> This fix is to require to include `Locale.ROOT` in the returned arrays/set > from `getAvailableLocales()` methods in various locale-sensitive classes. The > implementation has been including `Locale.ROOT` since its inception, it is > simply a doc clarification (+ a test case verifying it). Corresponding CSR > has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Using @code tag for `Set`. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6258/files - new: https://git.openjdk.java.net/jdk/pull/6258/files/1a611bd5..5bdd4917 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6258=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6258=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6258.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6258/head:pull/6258 PR: https://git.openjdk.java.net/jdk/pull/6258
Re: RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT
On Thu, 4 Nov 2021 16:07:01 GMT, Naoto Sato wrote:
> This fix is to require to include `Locale.ROOT` in the returned arrays/set
> from `getAvailableLocales()` methods in various locale-sensitive classes. The
> implementation has been including `Locale.ROOT` since its inception, it is
> simply a doc clarification (+ a test case verifying it). Corresponding CSR
> has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249
The change to the existing source looks good, and the new test looks beautiful.
Thanks for doing this.
src/java.base/share/classes/java/time/format/DecimalStyle.java line 118:
> 116: * Lists all the locales that are supported.
> 117: *
> 118: * At a minimum, the returned Set must contain a {@code Locale}
> instance equal to
A nit, really. Consider applying either of these suggestions:
Suggestion:
* At a minimum, the returned {@code Set} must contain a {@code Locale}
instance equal to
Suggestion:
* At a minimum, the returned set must contain a {@code Locale} instance
equal to
-
Marked as reviewed by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6258
RFR: 8276186: Require getAvailableLocales() methods to include Locale.ROOT
This fix is to require to include `Locale.ROOT` in the returned arrays/set from `getAvailableLocales()` methods in various locale-sensitive classes. The implementation has been including `Locale.ROOT` since its inception, it is simply a doc clarification (+ a test case verifying it). Corresponding CSR has also been drafted: https://bugs.openjdk.java.net/browse/JDK-8276249 - Commit messages: - Minor fixup - Reflected CSR modifications - Added a test case - 8276186: Include Locale.ROOT in getAvailableLocales() methods Changes: https://git.openjdk.java.net/jdk/pull/6258/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6258=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276186 Stats: 108 lines in 10 files changed: 86 ins; 0 del; 22 mod Patch: https://git.openjdk.java.net/jdk/pull/6258.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6258/head:pull/6258 PR: https://git.openjdk.java.net/jdk/pull/6258
