Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2012-02-28 Thread David Holmes
On 29/02/2012 10:29 AM, David Holmes wrote: Hi Joe, On 29/02/2012 3:03 AM, Joe Darcy wrote: Belatedly catching up on email, please review the patch below to address the issues you've raised. I searched for "method" and replaced it with "executable" as appropriate throughout the javadoc of the c

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2012-02-28 Thread David Holmes
Hi Joe, On 29/02/2012 3:03 AM, Joe Darcy wrote: Belatedly catching up on email, please review the patch below to address the issues you've raised. I searched for "method" and replaced it with "executable" as appropriate throughout the javadoc of the class. That substitution seems fine in the p

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2012-02-28 Thread Joe Darcy
On 2/28/2012 10:52 AM, Mike Duigou wrote: These changes look good to me. Is there a new CR for the javadoc changes? Thanks Mike; I was planning to file the bug after the reviews came in. -Joe Mike On Feb 28 2012, at 09:03 , Joe Darcy wrote: Hi David, Belatedly catching up on email, plea

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2012-02-28 Thread Mike Duigou
These changes look good to me. Is there a new CR for the javadoc changes? Mike On Feb 28 2012, at 09:03 , Joe Darcy wrote: > Hi David, > > Belatedly catching up on email, please review the patch below to address the > issues you've raised. I searched for "method" and replaced it with > "exe

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2012-02-28 Thread Joe Darcy
Hi David, Belatedly catching up on email, please review the patch below to address the issues you've raised. I searched for "method" and replaced it with "executable" as appropriate throughout the javadoc of the class. Thanks, -Joe --- a/src/share/classes/java/lang/reflect/Executable.java

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-20 Thread David Holmes
Just realized this has come in too late ... Joe Darcy said the following on 07/20/11 05:49: Agreed; I've posted a BlenderRev corresponding to the current patch at: http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html Thanks. So now I can more readily see that the doc for Executable

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-19 Thread Joe Darcy
On 7/19/2011 1:08 PM, Dr Andrew John Hughes wrote: On 12:49 Tue 19 Jul , Joe Darcy wrote: Agreed; I've posted a BlenderRev corresponding to the current patch at: http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html What is BlenderRev? Google finds others posted by Oracle empl

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-19 Thread Joe Darcy
Hi Mike. On 7/19/2011 1:54 PM, Mike Duigou wrote: I reviewed the BlenderRev fairly closely and did not find any errors. The only weirdness I saw was several cases where multiple "Specified by:" declarations were present for a method and one of the instances referenced a class to which it didn'

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-19 Thread Mike Duigou
I reviewed the BlenderRev fairly closely and did not find any errors. The only weirdness I saw was several cases where multiple "Specified by:" declarations were present for a method and one of the instances referenced a class to which it didn't appear to be able to link to. Example: Method.ge

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-19 Thread Dr Andrew John Hughes
On 12:49 Tue 19 Jul , Joe Darcy wrote: > Agreed; I've posted a BlenderRev corresponding to the current patch at: > > http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html > What is BlenderRev? Google finds others posted by Oracle employees but not how to generate them. > Thanks,

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-19 Thread Joe Darcy
Agreed; I've posted a BlenderRev corresponding to the current patch at: http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html Thanks, -Joe Mike Duigou wrote: This looks good to me but I agree with David that it's probably important to look at the generated javadoc and specdiff outpu

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-19 Thread Mike Duigou
This looks good to me but I agree with David that it's probably important to look at the generated javadoc and specdiff output before finalizing. Mike On Jul 18 2011, at 00:51 , Joe Darcy wrote: > Peter and David. > > Thanks for the careful review; the @throws information still needs its own

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-18 Thread David Holmes
Hi Joe, Seems okay. Can you use blenderrev (or specdiff or some other tool) to actually compare the generated javadoc output? One stylistic comment. The {@inheritDoc} in the main comment block of each method is superfluous as the default behaviour is to inherit those javadoc attributes ie th

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-18 Thread Joe Darcy
Peter and David. Thanks for the careful review; the @throws information still needs its own {@inheritDoc}; I've uploaded a webrev with this and other corrections: http://cr.openjdk.java.net/~darcy/7007535.4 More comments interspersed below. Thanks, -Joe Peter Jones wrote: Hi Joe, On Jul

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-16 Thread Peter Jones
Hi Joe, On Jul 15, 2011, at 1:49 AM, David Holmes wrote: > On 14/07/2011 12:21 PM, joe.da...@oracle.com wrote: >> Please code review my JDK 8 changes for >> >> 7007535: (reflect) Please generalize Constructor and Method >> http://cr.openjdk.java.net/~darcy/7007535.3 >> >> To summarize the change

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-14 Thread David Holmes
Hi Joe, On 14/07/2011 12:21 PM, joe.da...@oracle.com wrote: Please code review my JDK 8 changes for 7007535: (reflect) Please generalize Constructor and Method http://cr.openjdk.java.net/~darcy/7007535.3 To summarize the changes, a new superclass is defined to capture the common functionality

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-14 Thread Joe Darcy
Dr Andrew John Hughes wrote: On 19:21 Wed 13 Jul , joe.da...@oracle.com wrote: Hello. Please code review my JDK 8 changes for 7007535: (reflect) Please generalize Constructor and Method http://cr.openjdk.java.net/~darcy/7007535.3 To summarize the changes, a new superclass is de

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-14 Thread Dr Andrew John Hughes
On 19:21 Wed 13 Jul , joe.da...@oracle.com wrote: > Hello. > > Please code review my JDK 8 changes for > > 7007535: (reflect) Please generalize Constructor and Method > http://cr.openjdk.java.net/~darcy/7007535.3 > > To summarize the changes, a new superclass is defined to capture th

JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

2011-07-13 Thread joe . darcy
Hello. Please code review my JDK 8 changes for 7007535: (reflect) Please generalize Constructor and Method http://cr.openjdk.java.net/~darcy/7007535.3 To summarize the changes, a new superclass is defined to capture the common functionality of java.lang.reflect.Method and java.lang.refl