RFR: 8044609: javax.net.debug options not working and documented as expected [v10]

[Sean Coffey]

On Fri, 22 Nov 2024 19:07:34 GMT, Bradford Wetmore <wetmore at openjdk.org> wrote:

>> Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   enum Options and logic
>
> src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 101:
> 
>> 99:                 }
>> 100:             }
>> 101:             isOn = (property.isEmpty() || property.equals("all"))
> 
> What would you think about including `EMPTYALL` as an enum option, rather than a separate String property using separate processing checks?  This would simplify the logic, and you no longer need to store `property` in the class as it's no longer needed.  e.g.:
> 
>     isOn = activeComponents.contains(ComponentToken.EMPTYALL) 
>         || activeComponents.contains(ComponentToken.SSL));
> 
> This would help in the `isOn()` section, where you just check for `EMPTYALL`

the current isOn logic is pretty tight now also:


    public static boolean isOn(Opt option) {
        return Opt.ALL.on || option.on;
    }

> src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 154:
> 
>> 152:         }
>> 153: 
>> 154:         if (property.isEmpty() || property.equals("all")) {
> 
> Then here this becomes:
> 
>     if (activeComponents.contains(EMPTYALL)) {
>         return true;
>     }

design changed since this comment - what we have now is probably as easy to read:


    public static boolean isOn(Opt option) {
        return Opt.ALL.on || option.on;
    }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2352026377
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2352023953