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: Proposal For Inclusion of Robot and ParametersImpl in the Public API

2018-03-23 Thread Michael Ennen
Kevin, I believe I followed all of your suggestions, except the one with a re-usable WritableImage. If you want me to implement that as well, I can, but you seemed unsure about the necessity of it. Hopefully it is easy for you to review using the GitHub PR:

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 :

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

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