RFR: JDK-8232092 (fs) Files::isWritable returns false on a writeable root directory (win) [v9]

Alan Bateman alanb at openjdk.java.net
Wed Oct 7 18:24:12 UTC 2020


On Wed, 7 Oct 2020 17:29:56 GMT, Nhat Nguyen <github.com+8022952+nhat-nguyen at openjdk.org> wrote:

>> I see the latest version will attempt to find an unused driver letter. I'll try that out now in our infrastructure to
>> make sure that it is stable (Update: It passes on our infrastructure as it always find an unused drive so I think this
>> aspect of the patch is good).  substList returning a Stream is good but it should be Stream<String> rather than just
>> "Stream". Also the method name and comment suggests is returns a List, maybe it should just be renamed to mappedDrives
>> to be a bit clearer. The new findAvailableDrive is also missing the type variable, I think you want Optional<Path> here.
>
>> substList returning a Stream is good but it should be Stream rather than just "Stream". Also the method name and
>> comment suggests is returns a List, maybe it should just be renamed to mappedDrives to be a bit clearer. The new
>> findAvailableDrive is also missing the type variable, I think you want Optional here.
> 
> Thanks Alan for the feedback. I think there's a problem with how webrevs are being generated. I have been a bit
> confused by your comments on the usage of raw types because I actually have them from the beginning. I just double
> checked the webrevs and they are indeed missing. However, if you look at the Github diff, you will see that I'm using
> both `Stream<String>` and `Optional<Path>`. It also seems the comments on Github may also be incorrectly escaped, as
> `<whatever>` doesn't render unless I wrap it in backticks.

Indeed, it seems to be interpreting the tags as if they are HTML so looked to me that you were using raw types
everyone. Sorry I didn't notice this.

The updated changes look good and happy to sponsor.

-------------

PR: https://git.openjdk.java.net/jdk/pull/287


More information about the nio-dev mailing list