Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-11 Thread Lance Andersen
On Sat, 11 Sep 2021 13:59:25 GMT, Naoto Sato  wrote:

>> That sounds good to me, suggesting doing it at launch while discouraging the 
>> use of setProperty.
>
> Hi Lance,
> 
>> * Are there any scenarios where invoking setProperty will not override the 
>> command line setting ?
> 
> Yes. For example, if a logger is installed, it formats the log date/time 
> which causes initialization of locale-related classes before the app's main() 
> method invocation, resulting setProperty() having no effect.
> 
>> * Did you consider an `@ImplNote` for your clarification given the behavior 
>> "might" be different in other implementations (I am not sure myself) and is 
>> implementation defined?
> 
> As in the above example, the behavior changes regardless of the 
> implementation of the runtime. The clarification is mainly for the API 
> consumer, rather than platform implementors.
> 
> Hope this answers your questions.

Hi Naoto,

Thank you for the clarification.  Yes it makes sense.

Best
Lance

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-11 Thread Lance Andersen
On Fri, 10 Sep 2021 20:35:25 GMT, Naoto Sato  wrote:

>> Small spec clarification. Corresponding CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflecting review comment.

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-11 Thread Naoto Sato
On Fri, 10 Sep 2021 20:53:35 GMT, Joe Wang  wrote:

>> Hi Naoto,
>> 
>> A couple of questions:
>> 
>> - Are there any scenarios where invoking setProperty will not override the 
>> command line setting ?
>> - Did you consider an `@ImplNote` for your clarification given the behavior 
>> "might" be different in other implementations (I am not sure myself) and is 
>> implementation defined?
>> 
>> Best
>> Lance
>
> That sounds good to me, suggesting doing it at launch while discouraging the 
> use of setProperty.

Hi Lance,

> * Are there any scenarios where invoking setProperty will not override the 
> command line setting ?

Yes. For example, if a logger is installed, it formats the log date/time which 
causes initialization of locale-related classes before the app's main() method 
invocation, resulting setProperty() having no effect.

> * Did you consider an `@ImplNote` for your clarification given the behavior 
> "might" be different in other implementations (I am not sure myself) and is 
> implementation defined?

As in the above example, the behavior changes regardless of the implementation 
of the runtime. The clarification is mainly for the API consumer, rather than 
platform implementors.

Hope this answers your questions.

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-11 Thread Lance Andersen
On Fri, 10 Sep 2021 20:32:03 GMT, Naoto Sato  wrote:

>> src/java.base/share/classes/java/util/spi/LocaleServiceProvider.java line 
>> 120:
>> 
>>> 118:  * the locale sensitive services separated by a comma. It is only read 
>>> and cached at
>>> 119:  * the initialization of this class, so the later call to
>>> 120:  * {@link System#setProperty(String, String)} may not affect the order.
>> 
>> I wonder if we can be clearer as "may not" implies uncertainty. While it 
>> indeed may or may not work due to the timing of the initialization of this 
>> class, my understanding of the above statement is that it implied the 
>> runtime startup is recommended as it provides assurance. Would it be better 
>> to put that in the statement? sth. like: It is read once and cached at the 
>> Java runtime startup or initialization of this class. A call after the 
>> initialization of this class will not affect the order.
>
> It was intentional to use `may not` because as you said, there's still 
> uncertainty. To clarify it more, I added wording that `setProperty` use is 
> discouraged to change the preferred order.

Hi Naoto,

A couple of questions:

- Are there any scenarios where invoking setProperty will not override the 
command line setting ?
- Did you consider an `@ImplNote` for your clarification given the behavior 
"might" be different in other implementations (I am not sure myself) and is 
implementation defined?

Best
Lance

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-10 Thread Joe Wang
On Fri, 10 Sep 2021 20:32:03 GMT, Naoto Sato  wrote:

>> src/java.base/share/classes/java/util/spi/LocaleServiceProvider.java line 
>> 120:
>> 
>>> 118:  * the locale sensitive services separated by a comma. It is only read 
>>> and cached at
>>> 119:  * the initialization of this class, so the later call to
>>> 120:  * {@link System#setProperty(String, String)} may not affect the order.
>> 
>> I wonder if we can be clearer as "may not" implies uncertainty. While it 
>> indeed may or may not work due to the timing of the initialization of this 
>> class, my understanding of the above statement is that it implied the 
>> runtime startup is recommended as it provides assurance. Would it be better 
>> to put that in the statement? sth. like: It is read once and cached at the 
>> Java runtime startup or initialization of this class. A call after the 
>> initialization of this class will not affect the order.
>
> It was intentional to use `may not` because as you said, there's still 
> uncertainty. To clarify it more, I added wording that `setProperty` use is 
> discouraged to change the preferred order.

That sounds good to me, suggesting doing it at launch while discouraging the 
use of setProperty.

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-10 Thread Joe Wang
On Fri, 10 Sep 2021 20:35:25 GMT, Naoto Sato  wrote:

>> Small spec clarification. Corresponding CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflecting review comment.

Marked as reviewed by joehw (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-10 Thread Naoto Sato
On Fri, 10 Sep 2021 18:11:37 GMT, Joe Wang  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Reflecting review comment.
>
> src/java.base/share/classes/java/util/spi/LocaleServiceProvider.java line 120:
> 
>> 118:  * the locale sensitive services separated by a comma. It is only read 
>> and cached at
>> 119:  * the initialization of this class, so the later call to
>> 120:  * {@link System#setProperty(String, String)} may not affect the order.
> 
> I wonder if we can be clearer as "may not" implies uncertainty. While it 
> indeed may or may not work due to the timing of the initialization of this 
> class, my understanding of the above statement is that it implied the runtime 
> startup is recommended as it provides assurance. Would it be better to put 
> that in the statement? sth. like: It is read once and cached at the Java 
> runtime startup or initialization of this class. A call after the 
> initialization of this class will not affect the order.

It was intentional to use `may not` because as you said, there's still 
uncertainty. To clarify it more, I added wording that `setProperty` use is 
discouraged to change the preferred order.

-

PR: https://git.openjdk.java.net/jdk/pull/5457


Re: RFR: 8273491: java.util.spi.LocaleServiceProvider spec contains statement that is too strict [v2]

2021-09-10 Thread Naoto Sato
> Small spec clarification. Corresponding CSR has also been drafted.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Reflecting review comment.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5457/files
  - new: https://git.openjdk.java.net/jdk/pull/5457/files/2781ad32..70bfbd69

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5457=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5457=00-01

  Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5457.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5457/head:pull/5457

PR: https://git.openjdk.java.net/jdk/pull/5457