Hi Joe, here is the next version: http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/
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>(); > > do: > 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. > src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11D > TDScannerImpl.java: > > /* > > * 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 Christoph
