RFR 8144355: JDK 9 changes to ZipFileSystem to support multi-release jar files
Steve Drach
steve.drach at oracle.com
Mon Dec 21 17:55:44 UTC 2015
Hi,
I’ve updated the webrevs to take into account Paul’s and Alan's comments.
Issue: https://bugs.openjdk.java.net/browse/JDK-8144355 <https://bugs.openjdk.java.net/browse/JDK-8144355>
Change to modules.xml: <http://cr.openjdk.java.net/%7Esdrach/8144355/top/webrev.00/index.html>http://cr.openjdk.java.net/~sdrach/8144355/top/webrev.02/index.html <http://cr.openjdk.java.net/~sdrach/8144355/top/webrev.02/index.html>
Changes to ZipFileSystem: <http://cr.openjdk.java.net/%7Esdrach/8144355/jdk/webrev.01/index.html>http://cr.openjdk.java.net/~sdrach/8144355/jdk/webrev.02/index.html <http://cr.openjdk.java.net/~sdrach/8144355/jdk/webrev.02/index.html>
I removed the branch on System.property for windows specific functionality in MultiReleaseJarTest as Paul suggested.
I’ll address each of Alan’s concerns below:
> The changes to ZipFileSystem* looks okay although I wonder if getEntry0 should be renamed to getEntry as it's cleaner when overridden in JarFileSystem.
I did not change this. I’d like Sherman to comment on it. I assume getEntry0 was named for a reason.
> JarFileSystem looks okay and I assume the recursion in walk should never be an issue.
I assume so too since jar files have a finite, relatively small, set of entries.
> A minor nit is that is that it would be good to keep the style consistent with the existing code if you can. Really long source lines are annoying when looking at side-by-side diffs.
Consistent with ZipFileSystem? As for long lines, the longest line is 109 characters. I always try to keep lines shorter than 120 characters. I think it’s easier to read on a single line. Seems like a matter of style.
> Also good if the main comment in JarFileSystem could be expanded a bit, also the comments in the methods can use /** instead of /*.
I did that.
>
> The tests looks good. You might want to check the copyright header on MultiReleaseJarTest, the others look okay.
I don’t see what the issue is with the copyright.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20151221/aab336b0/attachment.html>
More information about the nio-dev
mailing list