Re: [9] Review request: 8178015: Clarify requirement for app modules to export/open packages to javafx modules

2017-04-20 Thread Mandy Chung 
+1
Mandy

> On Apr 20, 2017, at 11:06 AM, Kevin Rushforth  
> wrote:
> 
> Here is an updated webrev with a few suggested wording changes (e.g., removed 
> the reference to ModuleDescriptor, changed "accessible by" back to 
> "accessible to").
> 
> http://cr.openjdk.java.net/~kcr/8178015/webrev.02/
> 
> Additionally, I removed the example in the FXML annotation showing the use of 
> "opens to" in module-info.java (but left the example in Application).
> 
> -- Kevin
> 
> 
> Kevin Rushforth wrote:
>> 
>> 
>> Mandy Chung wrote:
 On Apr 18, 2017, at 12:48 PM, Kevin Rushforth  
 wrote:
 
 
 
 Alan Bateman wrote:
   
> On 18/04/2017 19:19, Kevin Rushforth wrote:
> 
>> Good suggestion. Here is an updated webrev with Mandy's suggestion and 
>> yours:
>> 
>> http://cr.openjdk.java.net/~kcr/8178015/webrev.01/
>> 
>> -- Kevin
>>
> This looks mostly okay.
> 
> I guess for FXML then I assume that the annotated member could be public, 
> in which case the package just needs to be exported (no need to open).
>  
 Yes, it could be, but the more typical use of the annotation is on 
 non-public members, so that was why I chose that as the example.

>>> 
>>> I have the same comment as Alan.  The example in the javadoc may be 
>>> perceived as a recommendation.  We should probably recommend the annotated 
>>> member be moved to public and encapsulated in its module, just exports it 
>>> to javafx.fxml to use.
>> 
>> For the use of the FXML annotation, that is exactly the recommendation. If 
>> you want the FXMLLoader to modify a public member in a public class, you 
>> don't need the annotation at all (although I guess it can still be used as 
>> useful documentation).
>> 
>> In the longer "Introduction to FXML" doc, which is included with the API 
>> docs and linked from the javafx.fxml package description, the only examples 
>> we give of using the @FXML annotation are for non-public members. See:
>> 
>> http://download.java.net/java/jdk9/jfxdocs/javafx/fxml/doc-files/introduction_to_fxml.html#fxml_annotation
>>  
>> 
>> Specifically, the description of the FXML annotation says:
>> 
>>   However, for developers who prefer more restricted visibility for
>>   controller fields or handler methods, the javafx.fxml.FXML
>>   annotation can be used. This annotation marks a protected or private
>>   class member as accessible to FXML.
>> 
>> 
>> For consistency, it seems best to show the example in the FXML javadocs as 
>> an example of using "opens" since that is what would be needed for the 
>> typical use case, such as those we document elsewhere.
>> 
>> -- Kevin
>> 
>> 
>>> Mandy

Re: [9] Review request: 8178015: Clarify requirement for app modules to export/open packages to javafx modules

2017-04-20 Thread Kevin Rushforth 
Here is an updated webrev with a few suggested wording changes (e.g., 
removed the reference to ModuleDescriptor, changed "accessible by" back 
to "accessible to").


http://cr.openjdk.java.net/~kcr/8178015/webrev.02/

Additionally, I removed the example in the FXML annotation showing the 
use of "opens to" in module-info.java (but left the example in Application).


-- Kevin


Kevin Rushforth wrote:



Mandy Chung wrote:
On Apr 18, 2017, at 12:48 PM, Kevin Rushforth 
 wrote:




Alan Bateman wrote:
   

On 18/04/2017 19:19, Kevin Rushforth wrote:
 
Good suggestion. Here is an updated webrev with Mandy's suggestion 
and yours:


http://cr.openjdk.java.net/~kcr/8178015/webrev.01/

-- Kevin


This looks mostly okay.

I guess for FXML then I assume that the annotated member could be 
public, in which case the package just needs to be exported (no 
need to open).
  
Yes, it could be, but the more typical use of the annotation is on 
non-public members, so that was why I chose that as the example.



I have the same comment as Alan.  The example in the javadoc may be 
perceived as a recommendation.  We should probably recommend the 
annotated member be moved to public and encapsulated in its module, 
just exports it to javafx.fxml to use.


For the use of the FXML annotation, that is exactly the 
recommendation. If you want the FXMLLoader to modify a public member 
in a public class, you don't need the annotation at all (although I 
guess it can still be used as useful documentation).


In the longer "Introduction to FXML" doc, which is included with the 
API docs and linked from the javafx.fxml package description, the only 
examples we give of using the @FXML annotation are for non-public 
members. See:


http://download.java.net/java/jdk9/jfxdocs/javafx/fxml/doc-files/introduction_to_fxml.html#fxml_annotation 



Specifically, the description of the FXML annotation says:

   However, for developers who prefer more restricted visibility for
   controller fields or handler methods, the javafx.fxml.FXML
   annotation can be used. This annotation marks a protected or private
   class member as accessible to FXML.


For consistency, it seems best to show the example in the FXML 
javadocs as an example of using "opens" since that is what would be 
needed for the typical use case, such as those we document elsewhere.


-- Kevin



Mandy

MarlinFX upgrade 0.7.5

2017-04-20 Thread Laurent Bourgès 
Hi,

Please review this MarlinFX upgrade to Marlin 0.7.5:

JBS: no bug yet for OpenJFX 10
webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-075.0/

Changes:
- Renderers: fixed block processing
- dead code & few comment removals in Strokers
- fixed all floating-point number literals to be x.0f or x.0d to simplify
the conversion between float & double variants

PS: I plan to run later FindBugs, Netbeans & IntelliJ code analysis tools
to fix any warning

Cheers,
Laurent