On Tue, 21 Oct 2025 18:52:03 GMT, Roger Riggs <[email protected]> wrote:

>> David Beaumont has updated the pull request with a new target base due to a 
>> merge or a rebase.
>
> src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java line 48:
> 
>> 46:     public static final int ATTRIBUTE_COMPRESSED = 6;
>> 47:     public static final int ATTRIBUTE_UNCOMPRESSED = 7;
>> 48:     public static final int ATTRIBUTE_PREVIEW_FLAGS = 8;
> 
> Please add a comment above these constants that the values are defined in 
> ImageFile.hpp.

Done. Thought really I'd say they are defined here, since this is what the code 
that creates the jimage file uses (so these constants are definitive). The C++ 
ones are copies for the C++ reading code.

> src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 57:
> 
>> 55:     /** If set, the associated module has resources (in normal or 
>> preview mode). */
>> 56:     // TODO: Make this private again when image writer code is updated.
>> 57:     public static final int FLAGS_HAS_CONTENT = 0x4;
> 
> Please change the prefix of these to distinguish them from the ImageLocation 
> flags.
> For example, "PKG_" or "MR_".

Done.

-------------

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1619#discussion_r2460165787
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1619#discussion_r2460317902

Reply via email to