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

Deadline for FX fixes for JDK 9 + FX 10-dev repo opening soon

[Kevin Rushforth]

All,

We are in the final week of bug fixing for JDK 9 prior to RDP2 [1]. All 
bug fixes targeted to JDK 9 must be done this week prior to the last 
integration for the RDP2 build.


If you have a safe (low-risk) fix for a P1-P3 bug, or a fix for any 
priority (P1-P5) test bug (label == 'testbug' or 'noreg-self') or doc 
bug (labels == 'noreg-doc') you can push it to FX 9-dev once the review 
is done with no additional approval needed, as long as it is pushed 
prior to the deadline.


The deadline for getting FX changes into 9-dev for the RDP2 build is 
*1:00 am Pacific on Friday, March 10*. Practically speaking this means 
you need to send the review out by Wednesday, March 8 afternoon Pacific 
unless it is a trivial fix and you can arrange for someone to review it.


No changesets should be pushed to FX 9-dev after the March 10, 1:00 am 
Pacific deadline without explicit approval, which will only be given for 
"must-fix" bugs. Bugs that are not "must-fix" should be deferred to JDK 
10 if they will not make the deadline.


I plan to open up the OpenJFX 10-dev repo later this week. Initially 
this will be for bug fixes that were deferred from JDK 9 and are not 
release stoppers. Stay tuned for more details.


-- Kevin

[1] http://openjdk.java.net/projects/jdk9/

[9] Review request for 8176236: Some field descriptions are truncated after the JDK-8163501

[Vadim Pakhnushev]

Kevin,
Could you please review this fix?

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

Thanks,
Vadim

Re: [9] Review request: 8170702: Document that javafx.graphics needs explicit access to application main class

[Kevin Rushforth]

Mandy Chung wrote:

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

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



Nit: @linkplain will show the normal font as text in javadoc rather than like the font for {@code…} 
  


Right...see my other reply. I'm happy to make this change if you plan to 
do the same for the rest of the JDK.




 209  * be a public subclass of Application, in a package that is
 210  * {@link Module#isExported(String,Module) exported} to at least the
 211  * {@code javafx.graphics} module, or a RuntimeException will be 
thrown.

Should it mention the public constructor?  No need to have a new webrev if you 
update this before you push.
  


A good point, yes, I will add this.

-- Kevin



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: 8170702: Document that javafx.graphics needs explicit access to application main class

[Kevin Rushforth]

Mandy Chung wrote:


On Mar 4, 2017, at 1:04 AM, Kevin Rushforth 
> wrote:




Mandy Chung wrote:

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

[fixed subject line]

Please review the following to document that javafx.graphics needs explicit 
access to the Application class.

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



  69  * containing package must be {@link Module#isExported(String,Module) 
exported}

@linkplain instead?
  


I was following the pattern in Module.java, etc., which uses a 
regular @link in similar cases.




Module.java should probably use @linkplain if that’s the case. 


At least Module.java and Layer.java use @link rather than @linkplain. I 
didn't do an exhaustive search. If you plan to file a bug to fix the JDK 
docs, then I will change it here to match (and in FXML.java for the 
other bug).


-- Kevin



 239 StackTraceElement[] cause = Thread.currentThread().getStackTrace();

Good candidate to use StackWalker API.
  


This is pre-existing code (since JDK 7), and I don't want to change 
the implementation this late while fixing a doc bug. I will file a 
follow-on bug to consider improving this for JDK 10.




It's fine for JDK 10.
Is @throws RuntimeException an existing behavior?  I’d think CNFE and 


Yes, this seems like another good place to document the restriction. 
I'll post a .01 version of the webrev with this update.


Sounds good.

Mandy