Hi Andy,
Looks good overall.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/Info-lite.plist.template.html
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/Info.plist.template.html
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/Runtime-Info.plist.template.html
All these files have:
<key>CFBundleSignature</key>
<string>????</string>
Based on doc "CFBundleSignature is the application’s creator signature,
a four-character code that identifies document files belonging to this
application."
Should it be unique per application? It seems that all bundles will have
same signature, not sure if it is correct.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/launchd.plist.template.html
Looks like old file from daemon support. Probably no longer needed.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties.html
Line 28 and 37 - Double space between sentences.
Some missing "." in some messages like line 32 and 34.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties.html
Line 47 - appRuntimeIMage -> appRuntimeImage
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/ResourceLocator.java.html
Do we need this file?
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/template.iss.html
Line 49-55 - Seems to be related to service support, which is not
currently supported. I think this file can be cleanup.
Minor issues that can be fixed as follow up cleanup task:
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebBundler.java.html
Line 305 - Remove if not needed.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxRpmBundler.java.html
Line 433 - Lets remove this TODO and file a bug if needed.
Line 587 - Remove TODO.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/linux/classes/jdk/jpackage/internal/resources/LinuxResources.properties.html
Line 27, 70 and 74 - No need for double space after ".".
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/linux/native/libapplauncher/LinuxPlatform.cpp.html
Line 864-871 - Remove if not needed.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppStoreBundler.java.html
Line 120 - Remove if not needed.
Line 314 - Remove.
Line 364-365 - Remove.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java.html
Line 521 and 524 - Remove
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/share/classes/jdk/jpackage/internal/StandardBundlerParam.java.html
Line 138 - Remove TODO
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/windows/native/libapplauncher/FileAttribute.h.html
Line 36 and 39 - Remove.
http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/src/jdk.jpackage/windows/native/libapplauncher/FilePath.cpp.html
Line 389-391 - Remove
Line 398-400 - Remove
Thanks,
Alexander
On 4/24/2019 5:47 PM, Andy Herrick wrote:
On 4/24/2019 8:44 PM, Andy Herrick wrote:
Please review changes for [1] which is the implementation bug for
JEP-343.
The webrev at [2] is the total cumulative webrev of changes for the
jpackage tool, currently in the JDK-8200758-branch branch of the open
sandbox repository.
The webrev at [3] shows the changes from EA-05 to EA-06.
sorry - the links are reversed from what is stated above. [2] is the
incremental webrev since EA 05, [3] is the cumulativewebrev
/Andy
The latest EA-6 (build 49) is posted at [4].
Please send feedback to core-libs-dev@openjdk.java.net
[1] https://bugs.openjdk.java.net/browse/JDK-8200758
[2] http://cr.openjdk.java.net/~herrick/8212780/webrev.05-06/
[3] http://cr.openjdk.java.net/~herrick/8212780/webrev.ea6/
[4] http://jdk.java.net/jpackage/
/Andy
