RFR 8211919: ZipDirectoryStream should provide a stream of paths that are relative to the directory
Langer, Christoph
christoph.langer at sap.com
Mon Jan 14 14:13:50 UTC 2019
Hi Lance,
as I'm currently active in that area, it's an easy one for me to review this
Overall the change looks good, thanks for doing it. Here are some few nits I discovered:
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java:
in the comments:
line 426: Maybe you could take the chance to improve the text of the comment ?
// (1) assume all path from zip file itself is "normalized" -> (1) assume each path from zip file is "normalized"
line 432: I think the word "also" can be removed
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java:
line 95: Could you please also fix the indentation of @Override (4 instead of 3 spaces)
test/jdk/jdk/nio/zipfs/Basic.java:
line 45: I think you should remove 8211385 from the @bug tag, since you removed the DirectoryStream tests from this file
test/jdk/jdk/nio/zipfs/ZipFsDirectoryStreamTests.java
I think you should add a @bug tag for both, 8211385 and 8211919
line 53: UNZIPFS_MAP: not needed
line 70: ; not needed in try statement
line 72: jarFile should be createad outside of try block
line 97: spelling: "filter"
line 106: space before + missing
line 144: ";" is not needed and bracket of try could be closed on this line
line 170: Spelling: use "a" instead of "an" ?
line 187: "an" instead of "a" ?
line 205: same ("an" instead of "a")
line 215: the assert for IllegalStateException could be dropped here, as it is a superclass of ClosedDirectoryStreamException which is asserted in line 219? Unless you want to use this as a test for ClosedDirectoryStreamException class hierarchy...
line 238: I think this commented debug code should be removed.
line 306: Looks like the matcher instance is not needed
line 309: you could use an import java.util.zip.ZipException?
line 314: The check should rather be: if (ds.iterator().hasNext()) throw...
Best regards
Christoph
> -----Original Message-----
> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On Behalf
> Of Lance Andersen
> Sent: Samstag, 12. Januar 2019 21:13
> To: core-libs-dev <core-libs-dev at openjdk.java.net>; nio-dev <nio-
> dev at openjdk.java.net>
> Subject: RFR 8211919: ZipDirectoryStream should provide a stream of paths
> that are relative to the directory
>
> Hi all,
>
> The following patch addresses the issue where the stream of paths where
> not relative to the specified directory as needed.
>
> As part of the fix, I added additional DirectoryStream tests similar to the tests
> in nio/file/DirectoryStream/Basic.java.
>
> Webrev can be found:
> http://cr.openjdk.java.net/~lancea/8211919/webrev.00/
>
> Best
> Lance
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>
>
More information about the core-libs-dev
mailing list