RFR 8211919: ZipDirectoryStream should provide a stream of paths that are relative to the directory
lance.andersen at oracle.com
Tue Jan 15 18:59:17 UTC 2019
> On Jan 15, 2019, at 4:07 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> On 15/01/2019 00:13, Lance Andersen wrote:
>> Thank you Alan pointing out this example which the previous fix also did not address. I updated the change which addresses the above example as well as the relative path
>> Please see http://cr.openjdk.java.net/~lancea/8211919/webrev.02/index.html
> Good, this looks right now. Are you planning to re-visit the 1-4 comments at L426-433, just asking because point 4 is confusing as there isn't any code here that cares if the path starts with "./" or “"
OK thank you. I made a ‘minor update to the comments and removed “4"
> I don't have time right now to study all the test cases in ZipFsDirectoryStreamTests so I only skimmed through it. I assume all the places that catch IOException and print the stack trace should be cleaned up to not catch the exception as you want the test to fail if it can't open the zip file.
I addressed this in each test
> Also I assume "Zipfs" can be dropped from the name to be consistent with the existing test classes.
Well I did that intentionally as one of the drawbacks with Intellij is running the tests within Intellij it gets awkward with too many tests using the same name. However for now I dropped the ZipFs from the test name
> A minor nit is that the really long lines will be annoying for future side-by-side reviews of changes to this test.
I pulled it into netbeans which makes it easier to see the width issues. Not sure if there is a way to enable similar functionality in Intellij?
Updated webrev is at http://cr.openjdk.java.net/~lancea/8211919/webrev.03/index.html
<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