Hi Christoph, > On Apr 26, 2019, at 12:50 PM, Langer, Christoph <christoph.lan...@sap.com> > wrote: > > Hi Lance, > > thanks for reviewing.
:-) > > As for ZipFSTester: the printf is anyway commented out, probably only used > for debugging. Removing the variable was just because the IDE said it’s > unused. I’d rather leave it as is. Well, i would tweak it or remove it. > > As for the jarfs tests: OK, it probably makes sense to separate these out > into a subfolder. So I’ll move MultiReleaseJarTest to the subfolder instead > of moving JFSTester. sounds like a plan > > OK for you if I push this without another webrev next week? Yes that is fine :-) Have a good weekend! Best Lance > > Best regards > Christoph > > From: Lance Andersen <lance.ander...@oracle.com > <mailto:lance.ander...@oracle.com>> > Sent: Freitag, 26. April 2019 18:26 > To: Langer, Christoph <christoph.lan...@sap.com > <mailto:christoph.lan...@sap.com>> > Cc: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net>; > nio-...@openjdk.java.net <mailto:nio-...@openjdk.java.net> > Subject: Re: RFR (S): 8223015: Cleanups for zipfs tests > > Hi Christoph, > > Thank you for doing this. Overall the changes look good. > > A few minor thoughts > > ZIpFSTester.java - 537: Given you removed the method variable, I would > tweak the printf string to make it a bit clearer that what the output its. > > JFSTester.java and MultiReleaseJarTest.java - I had the opposite feeling > that having a jarfs subfolder adds clarity and moving > MulltiReleaseJarTest.java might be the better way to go. It helps separate > specific tests which are using the JarFileSystem from the ZipFileSystem > especially as we add more, keeping things cleaner. > > Best > Lance > > > On Apr 26, 2019, at 8:35 AM, Langer, Christoph <christoph.lan...@sap.com > <mailto:christoph.lan...@sap.com>> wrote: > > Hi, > > please help reviewing these cleanups to the zipfs tests. In detail: > > - Fix warnings in test/jdk/jdk/nio/zipfs/Demo.java (not a real testcase but > Demo coding) > - change some calls of Runtime.version().major() to > Runtime.version().feature() (the former is deprecated) > - change occurences of new Integer() to Integer.valueOf() > - change occurences of class.newInstance() to > class.getDeclaredConstructor().newInstance() > - remove unused variables > - move test/jdk/jdk/nio/zipfs/jarfs/JFSTester.java to > test/jdk/jdk/nio/zipfs/JFSTester.java (not using the jarfs subfolder). There > is no need for this specific subfolder, given that MultiReleaseJarTest.java > neither isn't in the jarfs subfolder > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223015.0/ > <http://cr.openjdk.java.net/~clanger/webrevs/8223015.0/> > Bug: https://bugs.openjdk.java.net/browse/JDK-8223015 > <https://bugs.openjdk.java.net/browse/JDK-8223015> > > I verified that the tests still succeed with the change. > > Thanks > Christoph > > > <image001.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.ander...@oracle.com <mailto:lance.ander...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
