Hi Lance,

ok, then I’ll remove the commented printf in ZipFSTester altogether. In case 
anybody debugs this again, he or she will probably add their own printfs anyway.

Have a nice weekend, too 😊

/Christoph

From: Lance Andersen <lance.ander...@oracle.com>
Sent: Freitag, 26. April 2019 18:54
To: Langer, Christoph <christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net; nio-...@openjdk.java.net
Subject: Re: RFR (S): 8223015: Cleanups for zipfs tests

Hi Christoph,
On Apr 26, 2019, at 12:50 PM, Langer, Christoph 
<christoph.lan...@sap.com<mailto: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/
Bug: 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>






[cid:image001.gif@01D4FC62.D708F4F0]<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>




Reply via email to