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@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
