[modules-dev] Review request for 6560271 (embedded JAR files)

[Dave Bristor]

Thanks for the feedback, my followup inline.

Kumar Srinivasan wrote:
> Hi Dave,
> 
> Generally looks good. I am quite sure Stanley will do a more
> thorough review, here are some comments:
> 
> 1. I noticed you were using buffers of 4096 and other places
> 8192, I think we should put a static in JamUtils.BUFFER_SIZE = 8192
> and simply use this everywhere or if we want different sizes for
> whatever reason we could do this:
> JamUtils.LARGE_BUFFER_SIZE = 8192
> JamUtils.MEDIUM_BUFFER_SIZE = 4096
> JamUtils.SMALL_BUFFER_SIZE = 2048

I added a single BUFFER_SIZE = 8192 in JamUtils, and made use of it in files 
that are already part of this webrev.  I updated 6566724 to explicitly mention 
this for other code too:

6566724 Add constants for oft-used strings such as MODULE.METADATA

That'll make a nice cleanup task for a rainy day ;-)

> 2. LocalJamDefinitionContent.java s/cpde/code/g

Webrev updated:
http://analemma.sfbay.sun.com/java/jdk/ws/libs/rev/6560271

Fixed, thanks.

	Dave
> 
> 
> 
> 
>> Hi folks,
>>
>> This set of changes lets embedded legacy JAR files work, including with the 
>> JarLibraryPath annotation.  I plan to build on this in making changes to 
>> support native libraries.  Comments welcome!
>>
>> bugster: http://monaco.sfbay/detail.jsp?cr=6560271
>> webrev: http://analemma.sfbay.sun.com/java/jdk/ws/libs/rev/6560271
>>
>> Description of changes:
>>
>> Several bits of cleanup, such as for javadoc, consistency, etc.
>>
>> JamUtils: Added copyFile() which is therefore removed from URLRepository.
>>
>> LocalRepository: Improve location of expansionDir.
>>
>> URLRepository: Just cleanup.
>>
>> LocalJamDefinitionContent: Support for legacy/embedded JAR files.  Code 
>> cleanup to make it more consistent with URLModuleDefinitionContent.  I think 
>> this should be renamed to LocalModuleDefinitionContent for consistency (or 
>> rename the other class); opinions?
>>
>> URLModuleDefinitionContent: Support for legacy/embedded JAR files.
>>
>> RepositoryUtils: This new class has static methods which were formerly 
>> duplicated in others, especially {Local,URL}Repository.  In theory, this could 
>> be a pair of base classes, one for the *Repository implementations, and one 
>> for the *ModuleDefinition implementations.  The former is particularly likely.
>>
>> JamBuilder: now public constructor and can add annotations to the JAM that's 
>> to be built.
>>
>> LegacyJarTest: makes sure the above works ;-)
>>
>> Thanks,
>> 	Dave
>> _______________________________________________
>> modules-dev mailing list
>> modules-dev at openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/modules-dev
>>   
> 
>