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-boun...@openjdk.java.net> On Behalf
> Of Lance Andersen
> Sent: Samstag, 12. Januar 2019 21:13
> To: core-libs-dev <core-libs-dev@openjdk.java.net>; nio-dev <nio-
> d...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
>
>
