On Wed, 4 Dec 2024 16:40:42 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> 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.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22545#discussion_r1869926281
