On Wed, 13 Aug 2025 12:20:23 GMT, David Beaumont <d...@openjdk.org> wrote:
> This PR addresses several issues found while adding unit tests for
> ExplodedImage.
> I have added review comments for changes (some of which are a little
> preferential, but bring the code into line with things like ImageReader in
> terms of the name choices for variables).
> Later it is likely that this code will be adapted for the up-coming preview
> mode support in Valhalla, so having it unit-testable is important.
Looks pretty good.
src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java line 193:
> 191: * path references a file which must be hidden in the node
> hierarchy.
> 192: */
> 193: Node createModulesNode(String name, Path path) {
This and other methods could be `private` if not used outside the class.
src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java line 195:
> 193: Node createModulesNode(String name, Path path) {
> 194: assert !nodes.containsKey(name) : "Node must not already exist:
> " + name;
> 195: assert name.startsWith(MODULES) && name.length() >
> MODULES.length() : "Invalid modules name: " + name;
startsWith(MODUELES) && name.length(...) might be worth a utility method.
Only 2 uses though.
-------------
PR Review: https://git.openjdk.org/jdk/pull/26757#pullrequestreview-3141954854
PR Review Comment: https://git.openjdk.org/jdk/pull/26757#discussion_r2291906463
PR Review Comment: https://git.openjdk.org/jdk/pull/26757#discussion_r2291927933
