RFR (JAXP): 8054196: XPath: support any type

huizhe wang huizhe.wang at oracle.com
Tue Jan 20 18:51:50 UTC 2015


On 1/20/2015 7:02 AM, Lance Andersen wrote:
> Hi Joe,
>
> I think the changes look fine.

Thanks.
>
> I am wondering if we have any suggested standard for the use of 
> @Override  as I see it is inconsistent in its usage with methods 
> implementing an interface.  Is this something we should add to 
> existing code?  I don't see Netbeans asking for it to be done except 
> in the case of an overridden method?

NetBeans does make a suggestion such as: "Add @Override Annotation".  I 
think you're right about avoiding comment duplication. In case of 
existing "evaluate" methods, I previously updated* the javadocs for both 
the interface and impl. It makes sense to take advantage of the 
"automatically inherit" feature 
<http://docs.oracle.com/javase/7/docs/technotes/tools/solaris/javadoc.html#inheritingcomments> 
of the javadoc tool to avoid the duplication.

*Note that the update was only on format and styles, and some 
re-wording, no changes on definition or semantics.

>
> Thank you for the extra clean up Joe

Thank you, now it looks better and cleaner.

Best,
Joe

>
> Best
> Lance
> On Jan 16, 2015, at 9:33 PM, huizhe wang <huizhe.wang at oracle.com 
> <mailto:huizhe.wang at oracle.com>> wrote:
>
>>
>> On 1/16/2015 1:29 PM, Lance Andersen wrote:
>>> Hi Joe,
>>>
>>>
>>>
>>> Overall it is OK, a few minor comments
>>>
>>> -  Is there a reason that XPathExpressionImpl  is no longer public 
>>> and XPathImpl is public?
>>
>> Ok, I'll keep both public, may be useful in the future.
>>
>>> - I think you can leverage {@inheritdoc} in your impl classes to 
>>> avoid comment duplication possibly?
>>
>> Looks like javadoc "Automatically inherit comment ". So @Override is 
>> good enough. I've removed the javadocs for methods that override, 
>> including the existing methods. It's good to avoid the duplication, 
>> and potential copy n paste errors.
>>
>>> - please remember to make the copyright year 2015
>>
>> Done.
>>
>>> - XPathTestBase, can the static block be moved to a @BeforeClass
>>
>> Moved to within the Dataprovider.
>>
>>> - XPathNodes, should that have an @since 1.9?
>>
>> Fixed.
>>
>>> - Given you are not including @param, etc for your test comments, 
>>> you might want to consider /* */ vs /** */ style comments.
>>
>> Looks like s/\/**/\/* worked the trick.
>>
>>>  That is consider if you really want doc comments (really a style 
>>> choice but some IDEs will issue a warning for missing tags
>>
>> Yeh, it's good to turn off the warning "light" :-)
>>
>> http://cr.openjdk.java.net/~joehw/jdk9/8054196/webrev/
>>
>> Thanks,
>> Joe
>>
>>>
>>>
>>> HTH
>>>
>>> Best,
>>> Lance
>>> On Jan 16, 2015, at 2:51 PM, huizhe wang <huizhe.wang at oracle.com 
>>> <mailto:huizhe.wang at oracle.com>> wrote:
>>>
>>>> Hi all,
>>>>
>>>> Could you review the change?
>>>>
>>>> Thanks,
>>>> Joe
>>>>
>>>> On 12/18/2014 1:24 PM, huizhe wang wrote:
>>>>> Hi,
>>>>>
>>>>> This is to add support for any type and improvement with new 
>>>>> features reflected in the new evaluateExpression methods, 
>>>>> XPathEvaluationResult and XPathNodes.
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8054196
>>>>> http://cr.openjdk.java.net/~joehw/jdk9/8054196/webrev/
>>>>>
>>>>> Thanks,
>>>>> Joe
>>>>
>>>
>>> <Mail Attachment.gif> 
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
>>> Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>>>
>>>
>>>
>>
>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
> Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>
>
>




More information about the core-libs-dev mailing list