On Wed, 18 Feb 2026 18:02:38 GMT, Alan Bateman <[email protected]> wrote:
>> David Beaumont 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:
>>
>> - Merge branch 'lworld' into jdk_8377162_exploded_modules/squashed
>> - revert unnecessary filter in ModulePatcher
>> - Latest version
>> - Limit awareness of preview resources to system modules only.
>>
>> * limit preview handling
>> * Patch ModuleReaderTest from mainline
>> - Preview mode for exploded system module readers
>>
>> * with better flag plumbing
>> * Simpler approach with linked hash set
>> * Revert weird import change
>> * First cut at a preview aware module reader for exploded builds
>> * Test for partiy of JRT file-system and class resources in preview mode
>
> src/java.base/share/classes/jdk/internal/module/ModuleReferences.java line
> 376:
>
>> 374: ExplodedModuleReader(Path dir, boolean previewMode) {
>> 375: this.dir = dir;
>> 376: // Don't use PREVIEW_PREFIX here, as these paths may use
>> '\'.
>
> I'd prefer if this comment line were removed as it is confusing to see
> reference to PREVIEW_PREFIX further down. The portable way to access
> META-INF/preview is with `Path preview = dir.resolve("META-INF", "preview")`
> so the code is okay.
It's true that the code is okay, but since PREVIEW_PREFIX exists as a string, a
reader might think they could do `dir.resolve(PREVIEW_PREFIX)` which is what
this comment was pointing out wouldn't be correct.
If you feel it's not needed, I'll happily remove it.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2032#discussion_r2827238264
