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

2018-03-27 Thread mandy chung
@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

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

2018-03-27 Thread Kevin Rushforth
essage- 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

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

2018-03-27 Thread Tom Schindl
>> 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 modul

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

2018-03-27 Thread Ajit Ghaisas
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

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

2018-03-26 Thread Kevin Rushforth
-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

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

2018-03-26 Thread Kevin Rushforth
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

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

2018-03-26 Thread mandy chung
-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

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

2018-03-26 Thread Daniel Fuchs
Hi Ajit, Looks good to me. I obviously didn't review the build changes. best regards, -- daniel On 26/03/2018 12:36, 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.

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

2018-03-26 Thread Ajit Ghaisas
-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

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

2018-03-23 Thread Kevin Rushforth
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

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

2018-03-23 Thread Kevin Rushforth
mandy chung wrote: On 3/23/18 10:51 AM, Kevin Rushforth wrote: 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",

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

2018-03-23 Thread Daniel Fuchs
Hi Kevin, This sounds reasonable to me. best regards, -- daniel On 23/03/2018 17:51, Kevin Rushforth wrote: 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

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

2018-03-23 Thread 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

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

2018-03-23 Thread Daniel Fuchs
Hi Mandy, On 23/03/2018 17:00, mandy chung wrote: System::getLogger should return the same instance if it has been created. Not necessarily. j.u.l does that, but System::getLogger may return a new cheap wrapper. As you noted, if JavaFX sources only creates a handful of loggers, the cost of

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

2018-03-23 Thread 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

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

2018-03-23 Thread mandy chung
On 3/23/18 9:34 AM, 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 :