Hello Sherman,
The build changes look good to me.
/Erik
On 2014-04-14 19:52, Xueming Shen wrote:
Thanks Mandy and Alan for the review.
webrev has been updated accordingly to
(1) Removed the basic.sh. The tests are now using test.jdk to access
the zipfs.jar directly
(2) Updated most of the class to package package, only
ZipFileSystemProvider (the spi interface)
and ZipInfo are public.
The ZipInfo is a handy utility sometime I find it useful to
simply do java com.sun.nio.zipfs.Info xyz.zip
to display all cen and loc tables in details. I may take sometime
to find a better home for it later.
(3) I have not migrated the test target zipfs.jar to an "independent"
test source (created during test) yet,
will definitely later in a separate thread when we move to modules.
(4) Yes, I mean to keep Demo there as an interactive regression test.
http://cr.openjdk.java.net/~sherman/8038500/webrev/
-Sherman
On 4/11/14 4:29 PM, Mandy Chung wrote:
On 4/11/2014 3:42 PM, Xueming Shen wrote:
webrev: http://cr.openjdk.java.net/~sherman/8038500/webrev
It's good to see the source of the zip provider moved to the jdk
repo. You have made some public classes to package-private which is
good. I wonder whether a few remaining public classes can be made
package-private (e.g. ZipFileAttributes, ZipInfo etc). Is
JarFileSystemProvider used anywhere? ZipFileSystemProvider is the
only provider listed in the service configuration.
Do you mean to make Demo.java as a regression test?
basic.sh - I wonder if you can get rid of this shell test. Do Basic,
PathOps, ZipFSTester have any relationship? I was thinking if they
can be made as individual java test and constructor the input path of
zipfs.jar. With zipfs.jar part of the jdk build, it will be found
under ${java.home}/lib/ext/zipfs.jar. When we move to modules, this
jar file won't exist. It might be better for the tests to create a
JAR file (one to share by all zipfs tests) to get ready for modules.
Mandy
