On Wed, 4 Dec 2024 16:46:14 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> We're stuck with the property name for compatibility, and the usage within
>> the class is fairly limited.
>> Generally, it is easier to understand the behavior having a feature that is
>> enabled not a disable that is disabled.
>> $0.02
>
> Good :-)
>
> If you end up keeping the `JAR_CHECKING_ENABLED` name, then introducing a
> local variable for the evaluation of the property may be useful. It should
> also fit better with the comment you introduced:
>
>
> String p = props.getProperty("sun.misc.URLClassPath.disableJarChecking");
> // JAR check is disabled by default and will be enabled only if the "disable
> JAR check"
> // system property has been set to "false".
> boolean jarCheckingDisabled = "false".equals(p);
> JAR_CHECKING_ENABLED = !jarCheckingDisabled;
>
>
> But in my opinion the above just makes it explicit that we flip a boolean
> here just to flip it back again on the use site.
> We're stuck with the property name for compatibility, and the usage within
> the class is fairly limited. Generally, it is easier to understand the
> behavior having a feature that is enabled not a disable that is disabled.
> $0.02
Yes, agree that if we could start afresh, it would be better to disable
checking using "enableJarChecking=false". And yes, that ship has sailed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22545#discussion_r1869932859
