On Thu, 12 Dec 2024 09:23:32 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 500:
>>
>>> 498: * {@code roots} set.
>>> 499: */
>>> 500: public static ModuleFinder limitFinder(ModuleFinder finder,
>>
>> Inlining `limitFinder` sounds good while I suggest keeping the method name
>> as `newModuleFinder` which is clear.
>>
>> Suggestion:
>>
>> public static ModuleFinder newModuleFinder(ModuleFinder finder,
>
> Personally, I find it less clear to call it `newModuleFinder`. What it does
> is that it creates a finder based on the passed in finder that "finds" fewer
> modules. How about `newLimitedFinder()`? But no strong feelings, so if you
> think it should be `newModuleFinder` then it's fine by me.
I went with `newLimitedFinder`. Hope that's ok.
>> test/jdk/tools/jlink/IntegrationTest.java line 160:
>>
>>> 158: JlinkConfiguration config = new
>>> Jlink.JlinkConfiguration(output,
>>> 159: mods,
>>> 160:
>>> JlinkTask.limitFinder(JlinkTask.newModuleFinder(modulePaths), limits,
>>> mods), linkFromRuntime, false, false);
>>
>> `linkFromRuntime` parameter is no longer needed and this should fail
>> compilation.
>
> The parameter is also passed to `JlinkConfiguration` constructor. That still
> has the parameter.
I've re-formated the code to make this clearer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22609#discussion_r1881749570
PR Review Comment: https://git.openjdk.org/jdk/pull/22609#discussion_r1881754524
