Hi Magnus,

From a build perspective this looks fine.

Thanks for the review.

But it seems you are changing the interface for java.lang.System. Don't you need a CSR for that? Or is your claim that the interface was indeed changed by JDK-8197927, and it is a bug that the documentation was not updated to match this?

Yes, the second is true. The interface/spec was already changed with JDK-8197927 which also has a CSR attached. The commit is this: http://hg.openjdk.java.net/jdk/jdk/rev/d80becbcd3c1. What I'm modifying here is only the non API relevant doc of the private field props. The actual interface is documented in the Javadoc of method java.lang.System::getProperties(). Cheers Christoph

Hi Mandy, this is a good suggestion. The listing of system properties at the props field declaration seems somewhat redundant, given that it already exists more exactly and with API normativity in the doc of System::getProperties(). So what do you think of this version: http://cr.openjdk.java.net/~clanger/webrevs/8241947.1/ ? Thanks Christoph From: Mandy Chung <mandy.chung@oracle.com> Sent: Dienstag, 31. März 2020 19:34 To: Langer, Christoph <christoph.langer@sap.com>; core-libs-dev@openjdk.java.net Cc: build-dev <build-dev@openjdk.java.net> Subject: Re: RFR (S): 8241947: Minor comment fixes for system property handling On 3/31/20 7:56 AM, Langer, Christoph wrote: Hi, please review a small fix that updates two comments. The first one, in make/autoconf/spec.gmk.in, is probably quite old. It talks about handling of a property "vm.vendor" in VersionProps.java.template. However, there is no property "vm.vendor", it must rather be "java.vendor". I stumbled over that when looking at the change of JDK-4947890 (Minimize JNI upcalls in system-properties initialization). The second one is the code comment attached to "private static Properties props;" in java.lang.System. After JDK-8197927 (Allow the system property `java.vendor.version` to be undefined), "java.vendor.version" can be undefined, so this should be reflected in the comment. I also took the liberty to remove an unneeded import statement. Bug: https://bugs.openjdk.java.net/browse/JDK-8241947 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8241947.0/ I wonder if we still want to keep this block of comments listing a subset of system properties. Anyway the minor comment looks okay. Mandy

Thanks for the review, Alan. I'll push it then. Best regards Christoph

-----Original Message----- From: Alan Bateman <Alan.Bateman@oracle.com> Sent: Mittwoch, 1. April 2020 16:46 To: Langer, Christoph <christoph.langer@sap.com>; Mandy Chung <mandy.chung@oracle.com>; core-libs-dev@openjdk.java.net Cc: build-dev <build-dev@openjdk.java.net> Subject: Re: RFR (S): 8241947: Minor comment fixes for system property handling

On 31/03/2020 19:57, Langer, Christoph wrote:

...

Hi Mandy,

this is a good suggestion. The listing of system properties at the props field declaration seems somewhat redundant, given that it already exists more exactly and with API normativity in the doc of System::getProperties().

So what do you think of this version: http://cr.openjdk.java.net/~clanger/webrevs/8241947.1/ ?

Mandy's suggestion to link to getProperties instead is good. This version looks good to me.

-Alan.

On 01/04/2020 17:03, Roger Riggs wrote:

Hi,

Does dropping the "The following properties are guaranteed to be defined:" would seem to be a spec change. It's a comment on a private field, there's no change to the getProperties spec.

-Alan

+1 Mandy On 3/31/20 11:57 AM, Langer, Christoph wrote:

Hi Mandy,

this is a good suggestion. The listing of system properties at the props field declaration seems somewhat redundant, given that it already exists more exactly and with API normativity in the doc of System::getProperties().

So what do you think of this version: http://cr.openjdk.java.net/~clanger/webrevs/8241947.1/ <http://cr.openjdk.java.net/~clanger/webrevs/8241947.1/> ?

Thanks

Christoph

*From:* Mandy Chung <mandy.chung@oracle.com> *Sent:* Dienstag, 31. März 2020 19:34 *To:* Langer, Christoph <christoph.langer@sap.com>; core-libs-dev@openjdk.java.net *Cc:* build-dev <build-dev@openjdk.java.net> *Subject:* Re: RFR (S): 8241947: Minor comment fixes for system property handling

On 3/31/20 7:56 AM, Langer, Christoph wrote:

Hi,

please review a small fix that updates two comments.

The first one, in make/autoconf/spec.gmk.in, is probably quite old. It talks about handling of a property "vm.vendor" in VersionProps.java.template. However, there is no property "vm.vendor", it must rather be "java.vendor". I stumbled over that when looking at the change of JDK-4947890 (Minimize JNI upcalls in system-properties initialization).

The second one is the code comment attached to "private static Properties props;" in java.lang.System. After JDK-8197927 (Allow the system property `java.vendor.version` to be undefined), "java.vendor.version" can be undefined, so this should be reflected in the comment. I also took the liberty to remove an unneeded import statement.

Bug:https://bugs.openjdk.java.net/browse/JDK-8241947

Webrev:http://cr.openjdk.java.net/~clanger/webrevs/8241947.0/

I wonder if we still want to keep this block of comments listing a subset of system properties.  Anyway the minor comment looks okay.

Mandy