Looks good.
- Alexey
On 4/29/2020 2:36 PM, Andy Herrick wrote:
I don't think I sent out webrev.5 [6] fixing Alexander's points below.
Please Review:
[6] http://cr.openjdk.java.net/~herrick/8219536/webrev.05/index.html
/Andy
On 4/23/2020 7:59 PM, Alexander Matveev wrote:
Hi Andy,
http://cr.openjdk.java.net/~herrick/8219536/webrev.04/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/Arguments.java.frames.html
1) Copyright year needs to be updated. Other files also needs
copyright year to be updated.
2) Line 778: Not sure why it was moved to single line. I think it is
better to revert it back.
Otherwise looks fine.
Thanks,
Alexander
On 4/23/20 1:48 PM, Andy Herrick wrote:
Please review updated webrev at [5] to address comments below from
Alexey.
[5] http://cr.openjdk.java.net/~herrick/8219536/webrev.04
/Andy
On 4/23/2020 11:17 AM, Alexey Semenyuk wrote:
http://cr.openjdk.java.net/~herrick/8219536/webrev.03/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java.sdiff.html:731
- 'launcherName' parameter of readRuntimeReleaseFile() function
seems to be not used.
http://cr.openjdk.java.net/~herrick/8219536/webrev.03/test/jdk/tools/jpackage/share/jdk/jpackage/tests/JLinkOptionsTest.java.html:
copyright year should be 2020.
http://cr.openjdk.java.net/~herrick/8219536/webrev.03/test/jdk/tools/jpackage/share/jdk/jpackage/tests/JLinkOptionsTest.java.html:
128 - There is no point to assert if 'cfgfile' is not null. It
should be always non null for application packaging or
readLaunherCfgFile.
For testing if string contains some values, I'd suggest to use
TKit.assertTextStream():
---
List<String> mods = List.of(release.get(1));
if (required != null) {
for (String s : required) {
TKit.assertTextStream(s).label("mods").apply(mods.stream());
}
}
if (prohibited != null) {
for (String s : prohibited) {
TKit.assertTextStream(s).label("mods").negate().apply(mods.stream());
}
}
---
Will save you from maintaining explicit log messages.
- Alexey
On 4/23/2020 10:08 AM, Andy Herrick wrote:
Please review webrev at [1] to address issue [2].
This is the new feature to add the jpackage option --jlink-options
as specified in CSR at [3]
/Andy
[1] http://cr.openjdk.java.net/~herrick/8219536/webrev.03
[2] https://bugs.openjdk.java.net/browse/JDK-8219536
[3] https://bugs.openjdk.java.net/browse/JDK-8243272
