RFR 8218875: Add new FileSystems.newFileSystem methods
Lance Andersen
lance.andersen at oracle.com
Thu May 2 14:12:24 UTC 2019
Hi Roger,
Thank you for your review
> On May 2, 2019, at 9:35 AM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>
> Hi Lance,
>
> Can the edits in jdk.compiler replacing null with (ClassLoader)null use the 1 arg form of newFileSystem?
> Or are they needed for the version-1 bootstrap compilation?
No, I cannot use the 1 arg form unfortunately due to the bootstrap compilation. \
>
> Wildcard imports are discouraged (in the new test).
I thought I ran optimize imports in Intellij, but I can go back and do this. Thank you for catching this
>
> Test method names that are indicative of the function being tested can be helpful when
> reading a test log, it communicates a bit sooner what has gone wrong without having to read
> the test javadoc.
Well, i can change them if you prefer. Looking at the other tests in zipfs (ZipfsTester and in some of the other test areas, they are not very informative either
I tried to put a useful comment as to what the test does, but I will defer to your preference
Best
Lance
>
> Thanks, Roger
>
> On 05/02/2019 08:52 AM, Lance Andersen wrote:
>> Thank you Alan for catching the missing “method”
>>
>> http://cr.openjdk.java.net/~lancea/8218875/webrev.03/index.html <http://cr.openjdk.java.net/%7Elancea/8218875/webrev.03/index.html> contains your suggested updates mentioned below
>>
>> Best
>> Lance
>>> On May 2, 2019, at 3:40 AM, Alan Bateman <Alan.Bateman at oracle.com <mailto:Alan.Bateman at oracle.com>> wrote:
>>>
>>> On 01/05/2019 20:29, Lance Andersen wrote:
>>>> Coming back to this.
>>>>
>>>> The updated webrev can be found at:
>>>>
>>>> http://cr.openjdk.java.net/~lancea/8218875/webrev.02/index.html <http://cr.openjdk.java.net/%7Elancea/8218875/webrev.02/index.html>
>>>>
>>> I only have time to look at the changes to FileSystems.java right now.
>>>
>>> One nit in the wording is in "in exactly the same manner as the newFileSystem ..." - the word "method" has been dropped from the original text. Looks like the same thing shows up in several places so it would be good to fix those.
>>>
>>> Minor formatting nit on L472 where the "throws IOException" doesn't need to be on its own line.
>>>
>>> -Alan
>>>
>>>
>>>
>>
>> <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>
>>
>>
>>
>
<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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190502/54baf844/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190502/54baf844/oracle_sig_logo-0001.gif>
More information about the nio-dev
mailing list