JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE
huizhe.wang at oracle.com
Thu Feb 18 18:19:05 UTC 2016
On 2/18/2016 5:15 AM, Langer, Christoph wrote:
> Hi Joe,
> here is the next version:
Looks good! Thanks.
Copyright year in XSAttributeChecker.java is still incorrect. I meant to
say the year "2016" needs to have a comma, so in this case, it needs to
be "2015, 2016, " rather than "2015, 2016".
> Hopefully the final one :-)
>> Yes, it's good to have some cleanups. For the generic initialization,
>> it's preferable to have no redundant type arguments, for example,
>> instead of:
>> protected Stack<Entity> fEntityStack = new Stack<Entity>();
>> protected Stack<Entity> fEntityStack = new Stack<>();
>> Some of the "cleanup" in the webrev were reversing <> with added type
>> argument(s), that would be unnecessary.
> I think I did that on all the places that I touched now.
>> For the obsolete Vector, it would be good to replace it with ArrayList.
> There are so many Vector instances in this code that it's out of scope for me now to touch them all...
>> Copyright year "2016" in XSAttributeChecker needs to be "2016," or else
>> the script would complain.
> For that one I thought that it would look like "2015,2016" if the initial copyright was done in 2015. But I changed it as you suggested.
>> In XSDHandler, note that there's another call to get SECURITY_MANAGER
>> towards the end of the reset method (at 3572 in your new version) that
>> may be removed. You may also consolidate most of those
>> setFeature/Property statements in one try-catch block if you want.
> OK, I did some further refacturing here. However, I've left the try/catch blocks as I think it's the intention to silently catch some exceptions but then try to go on with the next property.
>> For the test, the @bug tag is still desirable in the general comment
>> section. For now, it's @bug 8149915. As we add more test to the class in
>> the future, we'd then append more bug ids, @bug 8149915, xxxxxxx.
> Done. I also left the @bug in the comments for the test method.
>>> I'm also wondering why some files have a copyright header like this, e.g.
>>> * reserved comment block
>>> * DO NOT REMOVE OR ALTER!
>>> Shouldn't there be the standard copyright header or is there some reason
>> behind it?
>> The block is used by the licensing script to swap with license
>> information required by licensees. For code that came from Xerces, we
>> should keep the block as is with the Apache License. But when we make
>> changes, we're required to put in the copyright header. Since the
>> changes you made are cleanup / removing unused fields, it's okay to keep
>> the block as is.
> OK :)
> Thanks again for reviewing.
> All the Best
More information about the core-libs-dev