Hi Joe, I think the changes look fine.
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? Thank you for the extra clean up Joe Best Lance On Jan 16, 2015, at 9:33 PM, huizhe wang <[email protected]> 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 <[email protected]> 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> >> >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> [email protected] >> >> >> > Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [email protected]
