[11] Review request : JDK-8195799 : Use System logger instead of platform logger in javafx modules

[Kevin Rushforth]

Hi Daniel,

Thanks for the review.

I like the idea of removing the unused levels and methods.

As for directly using System.Logger.Level, we have enough usages of the 
Level and convenience logging methods (e.g., "warn", "fine", etc.), that 
I think it's better to file a follow-up issue (to minimize the changes 
and so it would be more feasible to review the existing changes). The 
idea of doing a refactor / rename of the convenience methods and level 
names to match seems like a good one for that follow-up JBS issue.

-- Kevin


Daniel Fuchs wrote:
> Hi Ajit,
>
> I have two remarks,
>
> 1. I wonder if it's wise to keep the old unused levels like e.g.
>    Level.CONFIG in your new PlatformLogger.
>
>    The sun.util.logging.PlatformLogger had a bridge that allowed
>    it to transfer these levels unchanged to java.logging when
>    java.logging was the backend.
>
>    Your new PlatformLogger does not have these bridges.
>    Which means that code that might log with Level.CONFIG
>    will in reality log with Level.DEBUG, which will be translated
>    to Level.FINE when java.logging is the backend.
>
>    So with the old sun.u.l.PlatformLogger, logging with Level.CONFIG
>    would have ended up logging with Level.CONFIG in java.logging,
>    but with your new implementation, this will end up as Level.FINE
>    (as DEBUG maps to FINE when java.logging is the backend).
>
>    AFAICS - there's no code in JavaFX (at least in the files present
>    in your webrev) that logs with Level.CONFIG.
>    So why not just remove these unused levels from your implementation
>    of PlatformLogger? You certainly don't want new code
>    to start using PlatformLogger::config (as that's actually
>    an alias to DEBUG).
>    The best way of avoiding future usage when there's none
>    today is just to remove these problematic API point.
>
> 2. I understand the desire of minimizing the code changes, and
>    introducing a PlatformLogger class that mimics the old
>    sun.util.logging.PlatformLogger certainly achieve this goal.
>    Now the interesting thing is: how many log statements are there
>    throughout the JavaFX code base?
>
>    If there are not too many - then you might want consider
>    changing them to use directly the levels provided by
>    System.Logger - and then you might want to use the
>    "Refactor -> Rename" option of your IDE to simply
>    rename the methods
>
>    PlatformLogger::severe to PlatformLogger::error
>    PlatformLogger::fine to PlatformLogger::debug
>    PlatformLogger::finer to PlatformLogger::trace
>    PlatformLogger::finest to PlatformLogger::trace
>
>
> Otherwise looks mostly good - I guess.
>
> best regards,
>
> -- daniel
>
> On 23/03/2018 16:34, Ajit Ghaisas wrote:
>> Hi Kevin, Mandy and Daniel,
>>
>>      Please review the changeset that removes dependency on 
>> sun.util.logging package from JavaFX code.
>>
>>      Bug :  https://bugs.openjdk.java.net/browse/JDK-8195799
>>      Fix :  http://cr.openjdk.java.net/~aghaisas/fx/8195799/webrev.0/
>>
>>      Request you to review.
>>
>> Regards,
>> Ajit
>>
>