Re: RFR: JDK-8217894: jpackage CLI syntax changes

[Andy Herrick]

On 2/6/2019 11:06 AM, Alan Bateman wrote:



On 06/02/2019 15:53, Roger Riggs wrote:

Hi Andy,

In the help text,  the mention of "JImage" isn't needed. It will just 
make folks ask, what's a JImage.

Line 168:  Shrink it to: "separated list of modules to add"
Right, the jimage container is internal to the JDK so it shouldn't be 
in the usage output (only the `jimage` tool, included for 
troubleshooting purposes, should mention jimage files).


Andy - are you planning to sync up with JEP so that it documents the 
latest CLI options? I realize it's a pain to keep the two in sync 
while you iterate but it's also confusing to have the JEP document 
options that no longer exist.


yes - we had planed to sync up the JEP while these CLI and help message 
output changes were being implemented. I hope to get to that this week.

/Andy

-Alan

Re: RFR: JDK-8217894: jpackage CLI syntax changes

[Andy Herrick]

generally, these help text changes will be fixed in JDK-8217687 
 , where I have added 
line items from below.



On 2/6/2019 10:53 AM, Roger Riggs wrote:

Hi Andy,

In the help text,  the mention of "JImage" isn't needed. It will just 
make folks ask, what's a JImage.

Line 168:  Shrink it to: "separated list of modules to add"
ok, - added as line item 15 in JDK-8217687 
 .

do we want to more closely follow the java help text:

    --add-modules [,...]
  root modules to resolve in addition to the initial 
module.




The mention of "JLink" is probably a distraction too.
Instead:  "Path to look for modules when packaging the Java Runtime"

yes - it will go away


Do the language specific resource really need to be duplicated? Or is 
that just awaiting translation.

yes - until first translation we are asked to have all resources duplicated.


MainResources.properties(And the copies) the use of "force" in the 
keys should be changed to overwrite.

(And in the code too).

For completeness the JPackageCreateImageForceTest.java should be renamed.
I will make the above tow changes as part of JDK-8217687 



In the installer tests that add the --installer-type option, I'd put 
it on the same line with the EXT, etc.
so its clear they go together as is done with the other options with 
arguments.

also added to JDK-8217687 

/Andy


[Those installer tests would benefit from being data driven and there 
would be

fewer files to maintain.]

Thanks, Roger




On 02/05/2019 06:07 PM, Andy Herrick wrote:



On 2/5/2019 5:52 PM, Alexander Matveev wrote:

Hi Andy,

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java.frames.html 


Line 108 and 113 looks same. I think one of them can be removed.

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties.frames.html 

Line 84 - "Specifies the type of the installer created in 
create-image mode", I think it should be "created in 
create-installer mode".



yes to both - will fix.
/Andy


Otherwise looks fine.

Thanks,
Alexander

On 2/5/2019 8:06 AM, Andy Herrick wrote:

Please review the jpackage fix for [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


In response to internal and external feedback, the following syntax 
changes are being made to jpackage Command Line Interface (CLI):


1.) remove "create-jre-installer " mode, and add 
--runtime-installer option.
2.) remove the  arg from create-image mode, and add 
--installer-type option.

3.) rename the --class option to --main-class
4.) remove the --limit-modules option
5.) rename the --force option to --overwrite and improve it's help 
text.


[1] https://bugs.openjdk.java.net/browse/JDK-8217894

[2] http://cr.openjdk.java.net/~herrick/8217894/webrev.01/index.html

/Andy

Re: RFR: JDK-8217894: jpackage CLI syntax changes

[Alan Bateman]

On 06/02/2019 15:53, Roger Riggs wrote:

Hi Andy,

In the help text,  the mention of "JImage" isn't needed. It will just 
make folks ask, what's a JImage.

Line 168:  Shrink it to: "separated list of modules to add"
Right, the jimage container is internal to the JDK so it shouldn't be in 
the usage output (only the `jimage` tool, included for troubleshooting 
purposes, should mention jimage files).


Andy - are you planning to sync up with JEP so that it documents the 
latest CLI options? I realize it's a pain to keep the two in sync while 
you iterate but it's also confusing to have the JEP document options 
that no longer exist.


-Alan

Re: RFR: JDK-8217894: jpackage CLI syntax changes

[Roger Riggs]

Hi Andy,

In the help text,  the mention of "JImage" isn't needed. It will just 
make folks ask, what's a JImage.

Line 168:  Shrink it to: "separated list of modules to add"

The mention of "JLink" is probably a distraction too.
Instead:  "Path to look for modules when packaging the Java Runtime"

Do the language specific resource really need to be duplicated? Or is 
that just awaiting translation.


MainResources.properties(And the copies) the use of "force" in the keys 
should be changed to overwrite.

(And in the code too).

For completeness the JPackageCreateImageForceTest.java should be renamed.

In the installer tests that add the --installer-type option, I'd put it 
on the same line with the EXT, etc.
so its clear they go together as is done with the other options with 
arguments.


[Those installer tests would benefit from being data driven and there 
would be

fewer files to maintain.]

Thanks, Roger




On 02/05/2019 06:07 PM, Andy Herrick wrote:



On 2/5/2019 5:52 PM, Alexander Matveev wrote:

Hi Andy,

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java.frames.html 


Line 108 and 113 looks same. I think one of them can be removed.

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties.frames.html 

Line 84 - "Specifies the type of the installer created in 
create-image mode", I think it should be "created in create-installer 
mode".



yes to both - will fix.
/Andy


Otherwise looks fine.

Thanks,
Alexander

On 2/5/2019 8:06 AM, Andy Herrick wrote:

Please review the jpackage fix for  [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


In response to internal and external feedback, the following syntax 
changes are being made to jpackage Command Line Interface (CLI):


1.) remove "create-jre-installer " mode, and add 
--runtime-installer option.
2.) remove the  arg from create-image mode, and add 
--installer-type option.

3.) rename the --class option to --main-class
4.) remove the --limit-modules option
5.) rename the --force option to --overwrite and improve it's help 
text.


[1] https://bugs.openjdk.java.net/browse/JDK-8217894

[2] http://cr.openjdk.java.net/~herrick/8217894/webrev.01/index.html

/Andy

Re: RFR: JDK-8217894: jpackage CLI syntax changes

[Andy Herrick]

On 2/5/2019 5:52 PM, Alexander Matveev wrote:

Hi Andy,

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java.frames.html 


Line 108 and 113 looks same. I think one of them can be removed.

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties.frames.html 

Line 84 - "Specifies the type of the installer created in create-image 
mode", I think it should be "created in create-installer mode".



yes to both - will fix.
/Andy


Otherwise looks fine.

Thanks,
Alexander

On 2/5/2019 8:06 AM, Andy Herrick wrote:

Please review the jpackage fix for  [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


In response to internal and external feedback, the following syntax 
changes are being made to jpackage Command Line Interface (CLI):


1.) remove "create-jre-installer " mode, and add 
--runtime-installer option.
2.) remove the  arg from create-image mode, and add 
--installer-type option.

3.) rename the --class option to --main-class
4.) remove the --limit-modules option
5.) rename the --force option to --overwrite and improve it's help text.

[1] https://bugs.openjdk.java.net/browse/JDK-8217894

[2] http://cr.openjdk.java.net/~herrick/8217894/webrev.01/index.html

/Andy

Re: RFR: JDK-8217894: jpackage CLI syntax changes

[Alexander Matveev]

Hi Andy,

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java.frames.html
Line 108 and 113 looks same. I think one of them can be removed.

http://cr.openjdk.java.net/~herrick/8217894/webrev.01/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties.frames.html
Line 84 - "Specifies the type of the installer created in create-image 
mode", I think it should be "created in create-installer mode".


Otherwise looks fine.

Thanks,
Alexander

On 2/5/2019 8:06 AM, Andy Herrick wrote:

Please review the jpackage fix for  [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


In response to internal and external feedback, the following syntax 
changes are being made to jpackage Command Line Interface (CLI):


1.) remove "create-jre-installer " mode, and add 
--runtime-installer option.
2.) remove the  arg from create-image mode, and add 
--installer-type option.

3.) rename the --class option to --main-class
4.) remove the --limit-modules option
5.) rename the --force option to --overwrite and improve it's help text.

[1] https://bugs.openjdk.java.net/browse/JDK-8217894

[2] http://cr.openjdk.java.net/~herrick/8217894/webrev.01/index.html

/Andy