Re: Review Request JDK-8182416: Clean up module-info.java like move requires transitive adjacent to exports

2017-06-17 Thread Alan Bateman



On 17/06/2017 16:19, Mandy Chung wrote:

On Jun 17, 2017, at 7:26 AM, Alan Bateman  wrote:

On 17/06/2017 07:27, Mandy Chung wrote:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8182416/webrev.00/

This patch cleans up module-info.java mainly moving requires transitive
before exports, group exports/opens (packages) together and uses/provides
(services) next.

Document if a module is upgradeable.

jdeps —-generate-module-info is also updated to list requires transitive
before exports.


This looks okay to me, I just wonder if jdk.xml.ws and jdk.xml.bind modules 
need the upgradeable note given that they aren't standard modules.


Good point and will drop that sentence.
  

A minor nit in java.corba where it has "This ...". All the others have a 
space.


Fixed.


For jdeps then should it emit a blank line between the groupings to be 
consistent with the proposed layout?

Updated webrev:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8182416/webrev.01/


Looks good.

-Alan


Re: Review Request JDK-8182416: Clean up module-info.java like move requires transitive adjacent to exports

2017-06-17 Thread Mandy Chung

> On Jun 17, 2017, at 7:26 AM, Alan Bateman  wrote:
> 
> On 17/06/2017 07:27, Mandy Chung wrote:
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8182416/webrev.00/
>> 
>> This patch cleans up module-info.java mainly moving requires transitive
>> before exports, group exports/opens (packages) together and uses/provides
>> (services) next.
>> 
>> Document if a module is upgradeable.
>> 
>> jdeps —-generate-module-info is also updated to list requires transitive
>> before exports.
>> 
> This looks okay to me, I just wonder if jdk.xml.ws and jdk.xml.bind modules 
> need the upgradeable note given that they aren't standard modules.
> 

Good point and will drop that sentence.
 
> A minor nit in java.corba where it has "This ...". All the others have a 
> space.
> 

Fixed.

> For jdeps then should it emit a blank line between the groupings to be 
> consistent with the proposed layout?

Updated webrev:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8182416/webrev.01/

Mandy



Re: Review Request JDK-8182416: Clean up module-info.java like move requires transitive adjacent to exports

2017-06-17 Thread Alan Bateman

On 17/06/2017 07:27, Mandy Chung wrote:

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8182416/webrev.00/

This patch cleans up module-info.java mainly moving requires transitive
before exports, group exports/opens (packages) together and uses/provides
(services) next.

Document if a module is upgradeable.

jdeps —-generate-module-info is also updated to list requires transitive
before exports.

This looks okay to me, I just wonder if jdk.xml.ws and jdk.xml.bind 
modules need the upgradeable note given that they aren't standard modules.


A minor nit in java.corba where it has "This ...". All the others 
have a space.


For jdeps then should it emit a blank line between the groupings to be 
consistent with the proposed layout?


-Alan




Re: Review Request JDK-8182416: Clean up module-info.java like move requires transitive adjacent to exports

2017-06-17 Thread Lance Andersen
The changes looked fine Mandy.  Nice that everything is consistent 

best
lance
> On Jun 17, 2017, at 2:27 AM, Mandy Chung  wrote:
> 
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8182416/webrev.00/
> 
> This patch cleans up module-info.java mainly moving requires transitive
> before exports, group exports/opens (packages) together and uses/provides
> (services) next.
> 
> Document if a module is upgradeable.
> 
> jdeps —-generate-module-info is also updated to list requires transitive
> before exports.
> 
> Mandy

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com