I guess I’m going to keep doing this until I get it right ;-) Here’s another webrev that doesn’t use an exception for a common case, and addresses most of Mandy concerns.
http://cr.openjdk.java.net/~sdrach/8163798/webrev.06/ <http://cr.openjdk.java.net/~sdrach/8163798/webrev.06/> Comments in-line > This version looks good. > > Can you add the javadoc to describe what the stream method returns (a union > of the base entries + all versioned entries <= JarFile::getVersion. Done > > Nit: you may want to call jf.getVersion().major() once rather than the > number of entries. Done > > Now that you have jdk.internal.util.jar internal API for MRJAR. It may be > useful to add a static getRealName(JarFile jar, JarEntry entry) method for > convenience such that jdeps and jlink can use this MRJAR-specific internal > API and no need to use the shared secret. Maybe rename VersionedStream to > VersionedJarFileHelper? > > TestVersionedStream::close > You may want to use jdk.testlibrary.FileUtils and deleteFileTreeWithRetry > may be what you want. I can’t use it because it removes the top-level directory (i.e. JTwork/scratch). > > Mandy
