RFR: JDK-8134577 - Eliminate or standardize a replacement for sun.net.spi.nameservice.NameServiceDescriptor

[Mark Sheppard]

thanks for the feedback, Alan

based on suggestions, all issues have been addressed and patch has been 
updated

http://cr.openjdk.java.net/~msheppar/8134577/webrev.07

the following should be noted:

ExtensionsWithLDAP.java is added to the exclude list with JDK-8134577 - 
This is on the problem list as it will need to be re-written
The NameService associated with this test threw UnknownHostExceptions 
and added  the host name to a List, which was then examined
later in the test as a verification.

The instantiation of a NameService is as intended, i.e.
if jdk.net.hosts.file set and file exist then
instantiate HostsFileNameService
else
instantiate PlatformNameService

this is in keeping with previous initialization semantics - if the no 
service provider NameService
created then the default NameService was instantiated.

regards
Mark


On 05/01/2016 13:10, Alan Bateman wrote:
> On 05/01/2016 00:54, Mark Sheppard wrote:
>> Hi,
>>    as per feedback below webrev has been updated
>>
>> http://cr.openjdk.java.net/~msheppar/8134577/webrev.06/
>>
>> change summary:
>>
>> * List<NameService> nameServices replaced with Nameservice nameService
>>
>> * references to nameServices removed
>>
>> * private interface NameService added, with two implementation 
>> PlatformNameService and HostsFileNameService
>>
>> * Scanner created with UTF-8  charset
>>
>> * removed StringTokenizer, replaced with String.split()
>>
>> * comment handling extended, handling comments on line with mapping entry
>>
>> * try with resources added to addMappingToHostsFile method in tests
>
> I think this is starting to look good.
>
> A few small comments:
>
> - instantiateNameService (would createNameService be nicer?) still 
> falls back to PlatformNameService when the hosts file doesn't exist. 
> Is that expected?
>
> - HostsFileNameService.hostsFile can be final.
>
> - in removeComments then I assume you can just use indexOf and check 
> for -1 rather than contains + indexOf.
>
> - it would be nice if NameService had some javadoc. Also the methods 
> don't need to be declared public.
>
> - it would be nice if PlatformnameService and HostsFileNameService 
> could use /** */ style javadoc rather than //, just to keep the code 
> consistent.
>
> - ExtensionsWithLDAP.java is added to the exclude list with 
> JDK-8134577, is there is separate issue for this?
>
> - formatting. There are a few places where the indention is messed up 
> (5 spaces instead of 4 in a few places). Also some of the throws XXX 
> are intended in many different ways. Blank lines can help readability 
> but seem to be inconsistently used here. There's clearly been a lot of 
> revisions on this issue and maybe the simplest is to just reformat 
> this section of the file in the IDE.
>
> -Alan.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20160106/78a4bc31/attachment.html>