RFR - 8132734: java.util.jar.* changes to support multi-release jar files
Steve Drach
steve.drach at oracle.com
Fri Jan 22 23:10:29 UTC 2016
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
More information about the core-libs-dev
mailing list