On Wed, 14 May 2025 15:40:41 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix comment based on current behaviour.
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 95:
>
>> 93:
>> 94: private static final Set<PosixFilePermission> DEFAULT_PERMISSIONS =
>> 95: PosixFilePermissions.fromString("rwxrwxrwx");
>
> I am not convinced this needs to be changed as it becomes a personal style
> preference
Possibly not, but it is only used in one place, and anyone reading the code now
has the comment saying "If not specified in env, it will return 777." three
lines away from the code that uses "rwxrwxrwx" instead of nearly 300.
Plus, there isn't now a *mutable* static final constant which could be
accidentally leaked and mutated in the future.
Yes, it's personal preference to a degree, but I feel this is a small, but
non-zero, improvement in maintainability, and I would ask someone to do this if
I were reviewing a change which added this to the code.
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 113:
>
>> 111: final ZipCoder zc;
>> 112: private final ZipPath rootdir;
>> 113: // Start readOnly (safe mode) and maybe reset at end of
>> initialization.
>
> maybe -> may be
Both work. It "may be reset", but also "maybe we will reset it". I reworded
slightly.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2091678054
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2091679779
