On Wed, 4 Dec 2024 16:25:47 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> improve code comment
>
> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 89:
>
>> 87: // JAR check is disabled by default and will be enabled only if
>> the "disable JAR check"
>> 88: // system property has been set to "false".
>> 89: JAR_CHECKING_ENABLED = p != null && p.equals("false");
>
> I throw `BooleanArithmeticOverflowException` :-)
>
> I think this would be easier to follow if the static field name was aligned
> with the property name itself. It's easier if they have the same
> interpretation.
>
> So you could keep `DISABLE_JAR_CHECKING` and just do
>
> `DISABLE_JAR_CHECKING = !"false".equals(p)`
>
> and then just `if (DISABLE_JAR_CHECKING)` below.
>
> If you don't like that, at least consider that the above can be simplified to
> `"false".equals(p)`
Hello Eirik, I have received similar inputs in other places when discussing
this change. I myself had to think a few times on which naming to follow here
to make it easier to understand the code. I'll consider your and other inputs
and come back to this tomorrow afresh.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22545#discussion_r1869908682
