Re: modules versus SDK's

[Michael Dever]

Agreed.


On Mar 27, 2018, at 8:16 PM, Pedro Duque Vieira  
wrote:

Like Kevin says I don't think this is a one or the other choice.

I think we need to think about people who are just evaluating the platform
or learning, and whether making them also have to learn about build tools
is good.

I'd say part of the web's success is it shallow learning curve, and why
languages that are technically inferior like javascript are some times
preferred over technically superior languages.

I'd argue we should also have an installer to make the process of
evaluating/learning as easy as possible. We do need more javafx
programmers/adopters.

My 2 cents,

-- 
Pedro Duque Vieira

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

[mandy chung]

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
   
   

modules versus SDK's

[Pedro Duque Vieira]

Like Kevin says I don't think this is a one or the other choice.

I think we need to think about people who are just evaluating the platform
or learning, and whether making them also have to learn about build tools
is good.

I'd say part of the web's success is it shallow learning curve, and why
languages that are technically inferior like javascript are some times
preferred over technically superior languages.

I'd argue we should also have an installer to make the process of
evaluating/learning as easy as possible. We do need more javafx
programmers/adopters.

My 2 cents,

-- 
Pedro Duque Vieira

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

[Kevin Rushforth]

Yes it could be converted, but wasn't necessary to remove the depenency, 
so I filed a separate JBS issue to track this replacing the calls to 
j.u.l: JDK-8195974 .


-- Kevin


Tom Schindl wrote:

Minor question on this: I see the test stuff is using java.util.Logging
could this not be ported to PlatformLogger?

Tom

On 26.03.18 22:46, Kevin Rushforth wrote:
  

This looks fine to me now.

-- Kevin


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
  

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

[Tom Schindl]

Minor question on this: I see the test stuff is using java.util.Logging
could this not be ported to PlatformLogger?

Tom

On 26.03.18 22:46, Kevin Rushforth wrote:
> This looks fine to me now.
> 
> -- Kevin
> 
> 
> 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
>>>       

Re: Dependencies on java.desktop

[Tom Schindl]

On 27.03.18 14:26, Kevin Rushforth wrote:
> Hi Tom,
> 
> Yes, this is an unfortunate dependency. It is "only" an implementation
> dependency, meaning that nothing in the public API depends on
> java.desktop (which is why we don't "requires transient java.desktop"),
> so it should be possible to remove this dependency in the future. As
> noted, it is only there because Java Beans is part of the java.desktop
> module.
> 
> In the interim, your suggestion of "requires static java.base" could be
> the way to go. It would need a spec change to the JavaFX beans adapter
> classes documenting that they would throw an
> UnsupportedOperationException if java.desktop was not present at

How can that happen. To write a JavaBean the consumer of the API by
definition has to have java.desktop as a dependency how else would he've
been able to construct the bean?

> runtime, along with a recommendation that applications needing that
> functionality should add "requires java.desktop" to their own
> module-info.java.
> 
> Note that this would only help non-graphical JavaFX applications that
> use javafx.base for its collections, properties, and bindings, since
> javafx.graphics requires java.desktop in a way that currently cannot
> easily be made optional (not without reimplementing printing support
> anyway).
> 

I understand but I guess here one could do a spec change to define that
printing is only available if one add java.desktop yourself as a dependency.

Looking at printing I might be better moved to its own module, the
problem with JPMS is that you can't move stuff after having released the
API module, I wished instead of "require module" JPMS would have used
"import package".

Tom

Re: OpenJFX GitHub mirror

[Nir Lisker]

Kevin, can we get a label for this?

- Nir

On Mon, Mar 26, 2018 at 4:37 PM, Johan Vos  wrote:

> Hi Nir,
>
> About 4. (jfx-dev): you're right, I just removed that repository. That was
> just some testing before we did the real thing.
>
> As for the other points: I agree
>
> - Johan
>
> On Mon, Mar 26, 2018 at 12:03 PM Nir Lisker  wrote:
>
>> Hi All,
>>
>> A few comments about the mirror and JBS:
>>
>> 1. In PRs and issues on GitHub, I strongly suggest that the link to JBS be
>> included in the top comment. If the JBS issue was created after a
>> discussion, edit it in.
>>
>> 2. In JBS, I suggest to link to the GitHub mirror via More > Link > Web
>> Link and in the Link Text use something like "GitHub mirror" (open for
>> suggestions). JIRA renders the link in an easy to see way (easier than
>> looking at URLs). Iv'e tried it in a couple of issues, e.g.,
>> https://bugs.openjdk.java.net/browse/JDK-8198795 and it seems preferable
>> to
>> me.
>>
>> 3. In JBS, I suggest a new label will be used for issues which are linked
>> to GitHub for search purposes. This is similar to the webbug label.
>>
>> If these are agreeable, please add them to the contribution instructions.
>>
>> 4. What is https://github.com/javafxports/jfx-dev? Newcomers can confuse
>> it
>> with javafxports/openjdk-jfx.
>>
>> 5. When the mirror is fully ready and operational, we should advertise on
>> community pages (like Reddit) to gather up contributors. Please keep a
>> mental reminder when the time comes.
>>
>> Thanks to all who are working on this.
>>
>> - Nir
>>
>

Re: Dependencies on java.desktop

[Kevin Rushforth]

Hi Tom,

Yes, this is an unfortunate dependency. It is "only" an implementation 
dependency, meaning that nothing in the public API depends on 
java.desktop (which is why we don't "requires transient java.desktop"), 
so it should be possible to remove this dependency in the future. As 
noted, it is only there because Java Beans is part of the java.desktop 
module.


In the interim, your suggestion of "requires static java.base" could be 
the way to go. It would need a spec change to the JavaFX beans adapter 
classes documenting that they would throw an 
UnsupportedOperationException if java.desktop was not present at 
runtime, along with a recommendation that applications needing that 
functionality should add "requires java.desktop" to their own 
module-info.java.


Note that this would only help non-graphical JavaFX applications that 
use javafx.base for its collections, properties, and bindings, since 
javafx.graphics requires java.desktop in a way that currently cannot 
easily be made optional (not without reimplementing printing support 
anyway).


-- Kevin


Tom Schindl wrote:

Hi,

Anyone else has an opinion on that? Is require static the way to go?

Tom

On 21.03.18 23:23, Tom Schindl wrote:
  

Hi,

I always thought the JavaFX-Codebase should be able to run with just the
java.base module but I was browsing the codebase a bit and was suprised
(or rather shocked) that even the base-module requires java.desktop.

If I get it correct this because of the java.beans (provided by the
adapters) stuff is found in there. Why hasn't the requires there not
defined as:

requires static java.desktop;

Tom

Re: Dependencies on java.desktop

[Tom Schindl]

Hi,

Anyone else has an opinion on that? Is require static the way to go?

Tom

On 21.03.18 23:23, Tom Schindl wrote:
> Hi,
> 
> I always thought the JavaFX-Codebase should be able to run with just the
> java.base module but I was browsing the codebase a bit and was suprised
> (or rather shocked) that even the base-module requires java.desktop.
> 
> If I get it correct this because of the java.beans (provided by the
> adapters) stuff is found in there. Why hasn't the requires there not
> defined as:
> 
> requires static java.desktop;
> 
> Tom
> 

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

[Ajit Ghaisas]

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; openjfx-dev@openjdk.java.net
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:openjfx-dev@openjdk.java.net"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 :  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