Review request : JDK-8195799 : Use System logger instead of platform logger in javafx modules
daniel.fuchs at oracle.com
Fri Mar 23 17:21:01 UTC 2018
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.
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.
More information about the openjfx-dev