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.getTypeParameters(): Specified by: getTypeParameters in interface java.lang.reflect.GenericDeclaration It wasn't clear to me why it needed two "Specified by:" entries and only one of them was hot linked to the specifying class. Just javadoc weirdness? Mike On Jul 19 2011, at 12:49 , 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 > > 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 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 >>> {@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 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 changes, a new superclass is defined to capture the >>>>>> common >>>>>> functionality of java.lang.reflect.Method and >>>>>> java.lang.reflect.Constructor. >>>>>> That superclass is named "Executable" along the lines of >>>>>> javax.lang.model.ExecutableElement, which models constructors and >>>>>> methods in >>>>>> the JSR 269 language model. >>>>>> >>>>>> Both specification and implementation code are shared. To preserve the >>>>>> right >>>>>> @since behavior, it is common that in Method/Constructor the javadoc for >>>>>> a >>>>>> method will now look like: >>>>>> >>>>>> /** >>>>>> * {@inheritDoc} >>>>>> * @since 1.5 >>>>>> */ >>>>>> >>>>> Unless they have fixed/changed javadoc (entirely possible) it used to be >>>>> that the above would not cause @throws declarations for unchecked >>>>> exceptions to be inherited - you have/had to explicitly repeat them as: >>>>> >>>>> @throws <exception-type> {@inheritDoc} >>>>> >>>> Yes, that would seem to be needed for some of the inherited getters of >>>> generics info, which specify unchecked exception types. >>>> >>>> >>>>>> Since Executable is being created in JDK 8, it would be incorrect for >>>>>> methods in that class to have an @since of 1.5; adding the @since in >>>>>> Method/Constructor preserves the right information. >>>>>> >>>> In Executable.java, getAnnotation and getDeclaredAnnotations do have >>>> "@since 1.5"-- oversight? >>>> >>> Yes; that was incorrect. >>> >>> >>>> In Constructor.java and Method.java, getExceptionTypes has "@since 1.5", >>>> but that method has existed in those classes since 1.1. >>>> >>>> >>> Fixed. >>> >>> >>>> In Executable.java: >>>> >>>> 216 /** >>>> 217 * Returns an array of {@code Class} objects that represent the >>>> formal >>>> 218 * parameter types, in declaration order, of the method >>>> 219 * represented by this {@code Method} object. Returns an array of >>>> length >>>> 220 * 0 if the underlying method takes no parameters. >>>> 221 * >>>> 222 * @return the parameter types for the method this object >>>> 223 * represents >>>> >>>> At least "{@code Method}" needs to be generalized, and perhaps all >>>> occurrences of "method"? >>>> >>> Corrected. >>> >> >> >
