Hi Alan, et. al., I’ve released a new webrev that addresses all the issues you raised.
http://cr.openjdk.java.net/~sdrach/8132734/webrev.03/index.html <http://cr.openjdk.java.net/~sdrach/8132734/webrev.03/index.html> Specifically: > For Release then I have to admit that I dislike _9 and wonder if other > options were considered? javax.lang.model.SourceVersion uses the RELEASE_xx > convention for example. Changed to VERSION_9, i.e. Release.VERSION_9 > > Also I wonder about Release.ROOT and whether Release.UNVERSIONED was > considered? In general the phrase "root entry" in the javadoc makes me think > the root or top-most directory. An alternative that might be clearer is to > say "unversioned entry" and define that term clearly in the class description. Changed to BASE, i.e. Release.BASE > > The javadoc for Release.RUNTIME looks like it will have a javadoc link to > jdk.Version but that is a JDK-specific API. Could the text starting "The > effective runtime ..." move to an @implNote? Done > > I assume @since will be added to Release before this is pushed. Done > > The new JarFile ctor doesn't seem to handle the case that version is null > when multi release is forced. Also I assume it should specify @throws > SecurityException. Yes, both done and added more @throws clauses > > Could the legacy JarFile ctor be changed to this(file, verify, mode, > Release.ROOT) to avoid duplication? Done > > I don't have time to do a detailed pass over the updated tests but I wonder > if SimpleHttpServer is really a candidate to put in the testlibrary tree. It > looks like it is very specific to multi-release JARs and so I would expect to > be co-located with those tests rather than being a hazard in the testlibrary > tree. Moved SimpleHttpServer into the test that uses it, MultiReleaseJarHttpProperties > > A small comment is that it would be good to fix the really long lines before > these changes are pushed. This will help future side-by-side reviews where it > would be otherwise annoying to have to do horizontal scrolling to view diffs. I think I fixed this
