Hi Alan, Thanks for looking at this.
>> Please review the new webrev that addresses the issues raised by Sherman and >> Alan in the last iteration. In particular: >> - fixed the race condition in isMultiRelease() and another one with the >> variables ‘version’ and ‘configured’ >> - changed the fragment for JarURLConnection runtime versioning from >> ‘rtversioned’ to ‘runtime’, and created documentation for it >> - used try with resources to open JarFile in all the tests >> >> Issue: >> <https://bugs.openjdk.java.net/browse/JDK-8132734>https://bugs.openjdk.java.net/browse/JDK-8132734 >> <https://bugs.openjdk.java.net/browse/JDK-8132734> >> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 >> <https://bugs.openjdk.java.net/browse/JDK-8047305> >> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ >> <http://cr.openjdk.java.net/%7Epsandoz/multiversion-jar/jar-webrev/> >> > The updated webrev looks must better. In JarURLConnection then it would be > good if the reference to multi-release JARs should link to the description in > the JarFile spec. Sure, just put a “see JarFile” comment in? > > In the previous round then we were discussing renaming the > jdk.util.jar.multirelease property. Has there been any more on that? Less of a discussion than a suggestion ;-) I didn’t change it because I wanted to see how important it was — I figured if you didn’t comment it was okay. But you did comment, so I guess it’s important. I’ll change it. > > The test MultiReleaseJarURLConnection uses @library > /lib/testlibrary/java/util/jar so it's reaching across the file system to use > the infrastructure of the JarFile tests. It might be clearer to move the test > to the JarFile directory. I can do that. I didn’t think that was the right directory to put it in. Seems like it should be with the other JarURLConnection tests, that way I could just run jtreg on the directory and get all the JarURLConnection tests run, including the multi-release jar test. > > It would be nice if we could reduce some of the really long lines if > possible, just to make future side-by-side a bit easier (avoid horizontal > scrolling). I’ll try. > > -Alan.
