[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: [11] Review request : JDK-8195799 : Use System logger instead of platform logger in javafx modul
From:       mandy chung <mandy.chung () oracle ! com>
Date:       2018-03-28 3:32:21
Message-ID: 02d0cfe4-d860-ed2d-cb35-e998862afa31 () oracle ! com
[Download RAW message or body]

It can be addressed later while I think this is a simple change to the 
getInstance method.   I guess you prefer to keep PlatformLogger as close 
to the original version and that's fine.

Mandy

On 3/27/18 4:46 AM, Kevin Rushforth wrote:
> It doesn't seem harmful to keep the current implementation. This seems 
> better to leave for the follow-on JBS issue (JDK-8200236) unless there 
> something I am missing.
> 
> -- Kevin
> 
> 
> mandy chung wrote:
> > You don't need the loggers map and getLogger method can simply return
> > return new PlatformLogger(System.getLogger(name));
> > 
> > Other than this, looks fine.
> > 
> > Mandy
> > 
> > 
> > On 3/26/18 4:36 AM, Ajit Ghaisas wrote:
> > > Thanks all for the review.
> > > 
> > > I have addressed the review comments in \
> > > -http://cr.openjdk.java.net/~aghaisas/fx/8195799/webrev.1/ 
> > > The changes are -
> > > 1. Addressed the (c) year inaccuracies in files -
> > > modules/javafx.base/src/main/java/com/sun/javafx/collections/SetListenerHelper.java
> > >  modules/javafx.graphics/src/main/java/javafx/scene/Scene.java
> > > 
> > > 2. Removed tabs from \
> > > modules/javafx.base/src/test/java/test/com/sun/javafx/binding/SelectBindingTest.java
> > >  3. Removed unused methods from com.sun.javafx.logging.PlatformLogger class.
> > > 
> > > Also, I have created a new bug JDK-8200236 to address some of the valid \
> > > suggestions from Mandy and Daniel. 
> > > Request you to review the new webrev.
> > > 
> > > Regards,
> > > Ajit
> > > 
> > > -----Original Message-----
> > > From: Kevin Rushforth
> > > Sent: Saturday, March 24, 2018 3:27 AM
> > > To: Ajit Ghaisas
> > > Cc: Mandy Chung; Daniel Fuchs;openjfx-dev@openjdk.java.net
> > > Subject: Re: [11] Review request : JDK-8195799 : Use System logger instead of \
> > > platform logger in javafx modules 
> > > The only additional comments I have are couple typos and a white-space
> > > issue:
> > > 
> > > 1. There is a typo in the Copyright year (201 rather than 2018) in the \
> > > following two files: 
> > > modules/javafx.base/src/main/java/com/sun/javafx/collections/SetListenerHelper.java
> > >  modules/javafx.graphics/src/main/java/javafx/scene/Scene.java
> > > 
> > > 
> > > 2. There are tab characters in the following file that need to be changed to \
> > > spaces: 
> > > modules/javafx.base/src/test/java/test/com/sun/javafx/binding/SelectBindingTest.java
> > >  public static void setUpClass() {
> > > > > > System.err.println("SelectBindingTest : log messages are
> > > expected from these tests.");
> > > 
> > > 
> > > All my testing looks good. With this patch I am now able to run applications \
> > > with OpenJDK 10 + a standalone FX SDK with no qualified exports on the command \
> > > line (as long as it doesn't use Swing interop). 
> > > -- Kevin
> > > 
> > > 
> > > 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
> > > > 
> > > > 
> > 


[prev in list] [next in list] [prev in thread] [next in thread]