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

[Daniel Fuchs]

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
>