RFR 8218875: Add new FileSystems.newFileSystem methods

Roger Riggs Roger.Riggs at oracle.com
Thu May 2 14:27:05 UTC 2019


Hi Lance,

On 05/02/2019 10:12 AM, Lance Andersen wrote:
> Hi Roger,
>
> Thank you for your review
>> On May 2, 2019, at 9:35 AM, Roger Riggs <Roger.Riggs at oracle.com 
>> <mailto: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. \
ok
>
>>
>> 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
It depends on your Intellij configuration.  It can do many variations.  :)
See Editor -> Code Style -> Java and check "Use single class import"
>>
>> 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
I think its best practice to use useful names.
Future maintainers will appreciate it.

Thanks, Roger

>
> 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> 
>>> <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> <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> 
>>>>> <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> 
>>> <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/c3b6dab8/attachment.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/c3b6dab8/oracle_sig_logo.gif>


More information about the nio-dev mailing list