On 26 Apr 2016, at 09:20, Alan Bateman <alan.bate...@oracle.com> wrote:

> On 25/04/2016 22:01, Chris Hegarty wrote:
>> One of the remaining open issues in JEP 200 [1] is that the base module
>> exports the jdk.net package, thereby violating Principle 4 of JEP 200:
>> a Java SE module must not export any non-SE API packages without
>> qualification.
>> 
>> http://cr.openjdk.java.net/~chegar/8044773/
>> https://bugs.openjdk.java.net/browse/JDK-8044773
>> 
>> 
> I took a first pass over this and it looks very good.

Thanks Alan.

 Webrev updated in-place:
  http://cr.openjdk.java.net/~chegar/8044773/jdk/

> For the package name then sun.net.extnet repeats "net" and maybe sun.net.ext 
> would be better.

Agreed. Done.

> For the BaseExtendedSocketOptions then I think the FileDescriptor parameter 
> should be the first parameter as that is the file descriptor to the socket 
> that the methods operate on.

Agreed. Done.  I had it this way at one point, but I don’t remember why
I changed it.

> In ExtendedSocketOptions then I assume the toString is not needed in 
> checkValueType.

Right. Done. 

> There are "TODOs" in the tests and I assume it would be better to drop those 
> and instead create an issue in JIRA to be addressed once jtreg is promoted 
> with support for -limitmods.

Yes, this is better.
  https://bugs.openjdk.java.net/browse/JDK-8155086

> There is also a TODO in Sockets.java "check this is working ok" that I assume 
> needs attention before this is pushed.

Removed. Sorry, I thought I had removed this after I verified it was indeed
ok.

-Chris.

Reply via email to