Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Kevin Rushforth]

> I see.   I’m not close to this spec.  I am not sure if it worths 
further clarification such as “it is a public member”.  I’ll leave it up 
for you to decide.


I like this suggestion, so I will change it before I push.

Thanks.

-- Kevin


Mandy Chung wrote:


On Mar 6, 2017, at 7:11 AM, Kevin Rushforth 
> wrote:




Mandy Chung wrote:

On Mar 4, 2017, at 5:14 PM, Kevin Rushforth  wrote:

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




  40  * object {@link Module#isOpen opens} the containing package to the

Nit: s/@link/@linkplain

  41  * {@code javafx.fxml} module, either in its {@link ModuleDescriptor}
  42  * (module-info.class) or by calling {@link Module#addOpens}.

Do you intend to take out “(module-info.class)”?  
  


I was thinking to leave it in, since module-info.class is the most 
common way to specify a ModuleDescription. Maybe better would be:


@{link ModuleDescriptor} (e.g., in its module-info.class)



That’s okay.

  43  * An object is also reflectively accessible if it is declared as public,

Does “it” mean its constructor?
  


No. It means the declaration itself, for example:

@FXML
public String myString;


as opposed to:

@FXML
private String myString;


I see.   I’m not close to this spec.  I am not sure if it worths 
further clarification such as “it is a public member”.  I’ll leave it 
up for you to decide.


Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Mandy Chung]

> On Mar 6, 2017, at 7:11 AM, Kevin Rushforth  
> wrote:
> 
> 
> 
> Mandy Chung wrote:
>> 
>>> On Mar 4, 2017, at 5:14 PM, Kevin Rushforth  
>>>  wrote:
>>> 
>>> http://cr.openjdk.java.net/~kcr/8170701/webrev.01/ 
>>> 
>>> 
>>> 
>> 
>>   40  * object {@link Module#isOpen opens} the containing package to the
>> 
>> Nit: s/@link/@linkplain
>> 
>>   41  * {@code javafx.fxml} module, either in its {@link ModuleDescriptor}
>>   42  * (module-info.class) or by calling {@link Module#addOpens}.
>> 
>> Do you intend to take out “(module-info.class)”?  
>>   
> 
> I was thinking to leave it in, since module-info.class is the most common way 
> to specify a ModuleDescription. Maybe better would be:
> 
> @{link ModuleDescriptor} (e.g., in its module-info.class)
> 

That’s okay.
>>   43  * An object is also reflectively accessible if it is declared as 
>> public,
>> 
>> Does “it” mean its constructor?
>>   
> 
> No. It means the declaration itself, for example:
> 
> @FXML
> public String myString;
> 
> 
> as opposed to:
> 
> @FXML
> private String myString;

I see.   I’m not close to this spec.  I am not sure if it worths further 
clarification such as “it is a public member”.  I’ll leave it up for you to 
decide.

Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Kevin Rushforth]

Mandy Chung wrote:

On Mar 4, 2017, at 5:14 PM, Kevin Rushforth  wrote:

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




  40  * object {@link Module#isOpen opens} the containing package to the

Nit: s/@link/@linkplain

  41  * {@code javafx.fxml} module, either in its {@link ModuleDescriptor}
  42  * (module-info.class) or by calling {@link Module#addOpens}.

Do you intend to take out “(module-info.class)”?  
  


I was thinking to leave it in, since module-info.class is the most 
common way to specify a ModuleDescription. Maybe better would be:


   @{link ModuleDescriptor} (e.g., in its module-info.class)


  43  * An object is also reflectively accessible if it is declared as public,

Does “it” mean its constructor?
  


No. It means the declaration itself, for example:

   @FXML
   public String myString;


as opposed to:

   @FXML
   private String myString;

-- Kevin



  44  * is in a public class, and the module containing that class
  45  * {@link Module#isExported(String,Module) exports}

Nit: s/@link/@linkplain

  46  * the containing package to the {@code javafx.fxml} module.

This is word-smithing and formatting nit.  No need to send a new webrev.

Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Mandy Chung]

> On Mar 4, 2017, at 5:14 PM, Kevin Rushforth  
> wrote:
> 
> http://cr.openjdk.java.net/~kcr/8170701/webrev.01/
> 

  40  * object {@link Module#isOpen opens} the containing package to the

Nit: s/@link/@linkplain

  41  * {@code javafx.fxml} module, either in its {@link ModuleDescriptor}
  42  * (module-info.class) or by calling {@link Module#addOpens}.

Do you intend to take out “(module-info.class)”?  

  43  * An object is also reflectively accessible if it is declared as public,

Does “it” mean its constructor?

  44  * is in a public class, and the module containing that class
  45  * {@link Module#isExported(String,Module) exports}

Nit: s/@link/@linkplain

  46  * the containing package to the {@code javafx.fxml} module.

This is word-smithing and formatting nit.  No need to send a new webrev.

Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Kevin Rushforth]

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


Kevin Rushforth wrote:



Mandy Chung wrote:
On Mar 3, 2017, at 10:29 PM, Kevin Rushforth 
 wrote:


Please review the following to update the FXML docs to document the 
requirement for a module that annotates non-public members with 
@FXML to "open" the containing package to the javafx.fxml module.


https://bugs.openjdk.java.net/browse/JDK-8170701
http://cr.openjdk.java.net/~kcr/8170701/webrev.00/



The doc change looks fine to me.

Minor suggestion.  You may refer "module-info class” to {@link 
ModuleDescriptor} which can be synthesized (although it may not be 
common).
  


I'll make this change.

You may consider referencing {@link Module#isOpened} where you may 
say “the containing package of the object is {@link Module#isOpened 
opened} to {@code javafx.fxml} module.
  


Good idea I will do that.

I'll post an updated webrev.

-- Kevin


Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Kevin Rushforth]

Mandy Chung wrote:

On Mar 3, 2017, at 10:29 PM, Kevin Rushforth  wrote:

Please review the following to update the FXML docs to document the requirement for a 
module that annotates non-public members with @FXML to "open" the containing 
package to the javafx.fxml module.

https://bugs.openjdk.java.net/browse/JDK-8170701
http://cr.openjdk.java.net/~kcr/8170701/webrev.00/



The doc change looks fine to me.

Minor suggestion.  You may refer "module-info class” to {@link 
ModuleDescriptor} which can be synthesized (although it may not be common).
  


I'll make this change.


You may consider referencing {@link Module#isOpened} where you may say “the 
containing package of the object is {@link Module#isOpened opened} to {@code 
javafx.fxml} module.
  


Good idea I will do that.

I'll post an updated webrev.

-- Kevin


Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

[Mandy Chung]

> On Mar 3, 2017, at 10:29 PM, Kevin Rushforth  
> wrote:
> 
> Please review the following to update the FXML docs to document the 
> requirement for a module that annotates non-public members with @FXML to 
> "open" the containing package to the javafx.fxml module.
> 
> https://bugs.openjdk.java.net/browse/JDK-8170701
> http://cr.openjdk.java.net/~kcr/8170701/webrev.00/

The doc change looks fine to me.

Minor suggestion.  You may refer "module-info class” to {@link 
ModuleDescriptor} which can be synthesized (although it may not be common).

You may consider referencing {@link Module#isOpened} where you may say “the 
containing package of the object is {@link Module#isOpened opened} to {@code 
javafx.fxml} module.

Mandy

[9] Review request: 8170701: Update FXML documentation for setAccessible

[Kevin Rushforth]

Please review the following to update the FXML docs to document the 
requirement for a module that annotates non-public members with @FXML to 
"open" the containing package to the javafx.fxml module.


https://bugs.openjdk.java.net/browse/JDK-8170701
http://cr.openjdk.java.net/~kcr/8170701/webrev.00/

-- Kevin