Re: RFR: 8282819: Deprecate Locale class constructors [v4]
On Mon, 28 Mar 2022 17:13:44 GMT, Lance Andersen wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> New unit test. IllegalArgumentException.
>
> test/jdk/java/util/Locale/TestOf.java line 79:
>
>> 77: @Test (expectedExceptions = IllegalArgumentException.class)
>> 78: public void test_IAE() {
>> 79: Locale.of("en", "", "", "", "");
>
> I would consider using `assertThrows(IllegalArgumentException.class, () ->
> Locale.of("en", "", "", "", "")); ` instead of the expectedExceptions
> annotation element as it is the preferred way forward
Thanks. Modified as suggested.
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v4]
On Mon, 28 Mar 2022 17:02:33 GMT, Naoto Sato wrote:
>> Proposing to deprecate the constructors in the `java.util.Locale` class.
>> There is already a factory method and a builder to return singletons, so
>> there is no need to have constructors anymore unless one purposefully wants
>> to create `ill-formed` Locale objects, which is discouraged. We cannot
>> terminally deprecate those constructors for the compatibility to serialized
>> objects created with older JDKs. Please see the draft CSR for more detail.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> New unit test. IllegalArgumentException.
Changes look fine.
I would suggest adding a comment describing the new tests. Also one additional
suggestion below
test/jdk/java/util/Locale/TestOf.java line 79:
> 77: @Test (expectedExceptions = IllegalArgumentException.class)
> 78: public void test_IAE() {
> 79: Locale.of("en", "", "", "", "");
I would consider using `assertThrows(IllegalArgumentException.class, () ->
Locale.of("en", "", "", "", "")); ` instead of the expectedExceptions
annotation element as it is the preferred way forward
-
Marked as reviewed by lancea (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors [v4]
> Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: New unit test. IllegalArgumentException. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7947/files - new: https://git.openjdk.java.net/jdk/pull/7947/files/b4d040af..9d9d3a69 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7947&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7947&range=02-03 Stats: 87 lines in 2 files changed: 83 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7947.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7947/head:pull/7947 PR: https://git.openjdk.java.net/jdk/pull/7947
