Webrev updated in place.
> On Jun 15, 2016, at 9:50 AM, Jim Laskey (Oracle) <james.las...@oracle.com>
> wrote:
>
>
>> On Jun 14, 2016, at 11:51 PM, Mandy Chung <mandy.ch...@oracle.com> wrote:
>>
>> I reviewed http://cr.openjdk.java.net/~jlaskey/8159206/webrev
>>
>> Looks much cleaner. Thanks for the update
>>
>> 342 StringSharingPlugin(String[] patterns) throws IOException {
>> 343 this(ResourceFilter.includeFilter(Arrays.asList(patterns)));
>> 344 }
>>
>> Looks like it’s not used except the no-arg constructor. Perhaps just drop
>> this constructor and change the implementation of the no-arg constructor.
>
> will do
>
>>
>> 44 private static final String[] PATTERNS = {"*.diz”};
>>
>> I think this should be fixed too. The jdk build no longer packages *.diz
>> files in the jdk packaged modules. We should create a test to include *.diz
>> files to verify —-strip-native-debug and —-strip-debug plugins. It’s okay
>> to file a JBS issue to add that test.
>>
>
> will do
>
>>
>> I’m fine to separate the help message update from this patch. It looks like
>> it may need some iteration. This patch is looking good that you should push
>> soon.
>>
>
> will do
>
>> About the help message, you have this section in jlink/jimage/jmod:
>>
>> For options requiring a <pattern>, the argument value will be a comma
>> separated list of elements each using one the following forms:
>> <pattern> - a pattern string using the glob pattern syntax
>> glob:<pattern> - a pattern string using the glob pattern syntax
>> regex:<pattern> - a pattern string using the regex pattern syntax
>> @<filename> - the name of a file containing patterns to be use, one pattern
>> per line
>>
>> 1. It does not apply to jmod —-hash-modules <pattern> which is a regex.
>
> It does apply to -exclude. I’ll work it out of general comment and into the
> specific argument.
>
>> 2. It belongs to jlink —-list-plugins output but not jlink —-help.
>>
>
> will do
>
>
>> I have the impression that @<filename> is an undocumented option for the
>> build to use. Is it intended to document it?
>>
>
> It’s been there for some of the filters since the beginning. We should have
> it documented.
>
>> For jimage —help
>> --dir Target directory for extract directive
>> —-filter Filter [syntax:]<pattern> entries for list
>> or extract
>>
>> This should be
>> —dir <outdir> Target directory for extract directive
>> —-filter <pattern> Filter [syntax:]<pattern> entries for
>> list or extract
>>
>
> will do
>
>
>> Mandy
>
