> 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
