Thanks for the review. As suggested, I have added Mandy’s point about loggers map to be addressed as part of JDK-8200236.
Regards, Ajit From: Kevin Rushforth Sent: Tuesday, March 27, 2018 2:16 AM To: mandy chung Cc: Ajit Ghaisas; Daniel Fuchs; [email protected] Subject: Re: [11] Review request : JDK-8195799 : Use System logger instead of platform logger in javafx modules 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 - HYPERLINK "http://cr.openjdk.java.net/%7Eaghaisas/fx/8195799/webrev.1/"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; HYPERLINK "mailto:[email protected]"[email protected] 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 : HYPERLINK "http://cr.openjdk.java.net/%7Eaghaisas/fx/8195799/webrev.0/"http://cr.openjdk.java.net/~aghaisas/fx/8195799/webrev.0/ Request you to review. Regards, Ajit
