On Wed, 18 Oct 2023 06:26:24 GMT, Alan Bateman <[email protected]> wrote:
>> Calvin Cheung has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - comments from Alan and Ioi; add missing @run tag in the test
>> - Merge branch 'master' into improve-CDS-module-graph
>> - better way to check if a module is a JDK module
>> - initial review comments from Ioi
>> - 8316969: Improve CDS module graph support for --module option
>
> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line 472:
>
>> 470:
>> 471: // if -Xshare:dump and mainModule are specified, check if the
>> mainModule
>> 472: // is a builtin JDK module. If so, set canArchive to true so
>> that the
>
> There is no notion of "built-in JDK module" so I think change the comment to
> say that it checks if the main module is in the run-time image and not on the
> upgrade module path.
>
> URI schemes are compared without regard to case so it should be
> equalsIgnoreCase("jrt").
Fixed both.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1364683166
