Looks good.
- Alexey
On 4/30/2020 11:23 AM, Andy Herrick wrote:
Modified due to failure of new test on macosx. The relative location
of "release" file is different.
Please review revised fix [7]
/Andy
[7] - http://cr.openjdk.java.net/~herrick/8219536/webrev.06/
On 4/29/2020 5:01 PM, Alexey Semenyuk wrote:
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
