RFR: JDK-8221582: Rename jvm-args option to java-options

Alexander Matveev alexander.matveev at oracle.com
Thu Mar 28 21:26:24 UTC 2019


Hi Andy,

webrev.02 looks fine.

Thanks,
Alexander

On 3/28/2019 11:20 AM, Alexey Semenyuk wrote:
> Agreed. Would you like me to create a record to track this follow up 
> clean up?
>
> - Alexey
>
> On 3/28/2019 2:01 PM, Andy Herrick wrote:
>>
>>
>> On 3/28/2019 1:54 PM, Kevin Rushforth wrote:
>>> That seems like a good cleanup. It could be done in a follow-on bug 
>>> depending on what Andy wants to do.
>>>
>> Yes - I think we should file this as a follow on fix.
>> My main concern first was the public facing names, but as with other 
>> modified options, it would be best to go back and clean all internal 
>> names to reflect the current naming.
>>
>> /Andy
>>
>>> -- Kevin
>>>
>>>
>>> On 3/28/2019 10:50 AM, Alexey Semenyuk wrote:
>>>> Andy,
>>>>
>>>> IMHO if you are renaming defines in C++ code, it makes sense to 
>>>> rename variables/functions too:
>>>> JVMArgs -> JavaArgs in 
>>>> http://cr.openjdk.java.net/~herrick/8221582/webrev.02/src/jdk.jpackage/share/native/libapplauncher/Helpers.cpp.sdiff.html
>>>> Package::ReadJVMArgs -> Package::ReadJavaArgs in 
>>>> http://cr.openjdk.java.net/~herrick/8221582/webrev.02/src/jdk.jpackage/share/native/libapplauncher/Package.cpp.sdiff.html
>>>>
>>>> In general it would make sense to rename JVMArgs in JavaArgs:
>>>> ---
>>>> ASEMENYU-LAP+asemenyu at ASEMENYU-LAP 
>>>> /cygdrive/c/ade/work/as/jds/work/10_sandbox/jdk10/open/src/jdk.jpackage 
>>>>
>>>> $ find . -name '*.cpp' -o -name '*.h' | xargs.exe grep JVMArgs
>>>> ./share/native/libapplauncher/Helpers.cpp: 
>>>> Helpers::GetJVMArgsFromConfig(IPropertyContainer* config) {
>>>> ./share/native/libapplauncher/Helpers.cpp: OrderedMap<TString, 
>>>> TString> JVMArgs =
>>>> ./share/native/libapplauncher/Helpers.cpp: 
>>>> Helpers::GetJVMArgsFromConfig(&propertyFile);
>>>> ./share/native/libapplauncher/Helpers.cpp: 
>>>> Container->AppendSection(keys[CONFIG_SECTION_JVMOPTIONS], JVMArgs);
>>>> ./share/native/libapplauncher/Helpers.h: 
>>>> GetJVMArgsFromConfig(IPropertyContainer* config);
>>>> ./share/native/libapplauncher/JavaVirtualMachine.cpp: 
>>>> options.AppendValues(package.GetJVMArgs());
>>>> ./share/native/libapplauncher/Package.cpp: ReadJVMArgs(config);
>>>> ./share/native/libapplauncher/Package.cpp:void 
>>>> Package::ReadJVMArgs(ISectionalPropertyContainer* Config) {
>>>> ./share/native/libapplauncher/Package.cpp: FBootFields->FJVMArgs);
>>>> ./share/native/libapplauncher/Package.cpp: FBootFields->FJVMArgs);
>>>> ./share/native/libapplauncher/Package.cpp: FBootFields->FJVMArgs);
>>>> ./share/native/libapplauncher/Package.cpp:OrderedMap<TString, 
>>>> TString> Package::GetJVMArgs() {
>>>> ./share/native/libapplauncher/Package.cpp:    return 
>>>> FBootFields->FJVMArgs;
>>>> ./share/native/libapplauncher/Package.h: OrderedMap<TString, 
>>>> TString> FJVMArgs;
>>>> ./share/native/libapplauncher/Package.h:    void 
>>>> ReadJVMArgs(ISectionalPropertyContainer* Config);
>>>> ./share/native/libapplauncher/Package.h: OrderedMap<TString, 
>>>> TString> GetJVMArgs();
>>>> ---
>>>>
>>>> - Alexey
>>>>
>>>> On 3/28/2019 7:07 AM, Andy Herrick wrote:
>>>>> RFR: JDK-8221582: Rename jvm-args option to java-options
>>>>>
>>>>> Please review the jpackage fix for bug [1] at [2].
>>>>>
>>>>> This is a fix for the JDK-8200758-branch branch of the open 
>>>>> sandbox repository (jpackage).
>>>>>
>>>>> [1] - https://bugs.openjdk.java.net/browse/JDK-8221582
>>>>>
>>>>> [2] - http://cr.openjdk.java.net/~herrick/8221582/
>>>>>
>>>>> /Andy
>>>>
>>>
>>
>



More information about the core-libs-dev mailing list