Hi Joe, I fixed the copyright in http://cr.openjdk.java.net/~clanger/webrevs/8149915.3/. So I think I'm done. Can you review and push the change then?
Thanks Christoph -----Original Message----- From: huizhe wang [mailto:huizhe.w...@oracle.com] Sent: Donnerstag, 18. Februar 2016 19:19 To: Langer, Christoph <christoph.lan...@sap.com> Cc: core-libs-dev@openjdk.java.net Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE On 2/18/2016 5:15 AM, Langer, Christoph wrote: > Hi Joe, > > here is the next version: > http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/ 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". Best, Joe > > 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 > >
