Review Request: 8173381: osName/osArch/osVersion is missing in	ModuleDescriptor created by SystemModules
    Mandy Chung 
    mandy.chung at oracle.com
       
    Sat Jan 28 20:50:44 UTC 2017
    
    
  
Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173381/webrev.02
> On Jan 28, 2017, at 10:01 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 27/01/2017 06:21, Mandy Chung wrote:
> 
>> :
>> Updated webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173381/webrev.01/
>> 
> This mostly looks good.
> 
> I think it would cleaner if the plugin used rewriter.targetPlatform("", "", "") rather than adding filtering to the extender.
> 
I agree that would be cleaner and made the change. That’s what I started with but that’d leave ModuleTarget attribute in the class file. 
> For the secret option for testing then I assume it should be "retainModuleTarget" rather than "retainTargetPlatform". There are a couple of places that use the old name in method names and maybe we should rename those too. One other rename is the plugin has "PackagesAttribute" when it should be "ModulePackages" attribute.
I fixed up a few places in ModuleInfoExtender as well.
Mandy
    
    
More information about the jigsaw-dev
mailing list