RFR: JDK-8219536: Add Option for user defined jlink options
Alexey Semenyuk
alexey.semenyuk at oracle.com
Thu Apr 30 22:50:18 UTC 2020
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
>>>>>>>
>>>>>>
>>>>
>>
More information about the core-libs-dev
mailing list