On Mon, 4 Dec 2023 04:43:46 GMT, David Holmes <dhol...@openjdk.org> wrote:

>> src/hotspot/share/prims/jvmti.xml line 746:
>> 
>>> 744:     <code>JNI_CreateJavaVM</code> (in the JNI Invocation API) will 
>>> prepend these options to the options supplied
>>> 745:     in its <code>JavaVMInitArgs</code> argument. Note that module 
>>> related options must be expressed in their
>>> 746:     "option=value" form (not "option value") for 
>>> <code>JNI_CreateJavaVM</code> to process them correctly.
>> 
>> This looks okay. I'm just comparing it to the text that we put into the JNI 
>> spec:
>> 
>> "The module related options ... as option strings using their "option=value" 
>> format instead of their "option value" format. (Note the required = between 
>> "option" and "value".)"
>> 
>> It uses "format" instead of "form" and also, the bit I think works well, is 
>> to point out "required =" to force the reader to re-read the previous 
>> sentence and see what the difference is in the formats.
>
> Thanks for looking at this @AlanBateman  and @sspitsyn . I was trying to 
> refer to the JNI text in a casual way rather than duplicating the actual 
> content from there. This was more a strong hint/suggestion to "go read the 
> JNI spec for details".
> 
> Happy to change 'form' to 'format'.

I agree with not duplicating the entire sentence but what would you think about 
replacing the text in the parenthesis  with "Note the required = between option 
and value". Only asking because "(not "option value")" isn't as clear that we 
want the reader to see that you can't use a space here.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16896#discussion_r1413811389

Reply via email to