One more thing: the warning message should be in plugin.properties to be localized.
Mandy > On Feb 16, 2017, at 12:04 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: > > >> On Feb 16, 2017, at 5:20 AM, Claes Redestad <claes.redes...@oracle.com> >> wrote: >> >>> >>> In addition, if the main argument is specified but the version does not >>> match, it will ignore the given argument. Should it be an error instead? >>> We are the one who will generate a trace file and specify it in the jlink >>> plugin option. It’s okay to ignore the default trace output if no plugin >>> option is specified and I think no warning should be printed in this case. >>> It’s just like this plugin is disabled. You may want to add a suboption to >>> turn on verbose that will trace what is generated and what is ignored. >> >> I think a warning is reasonable in all cases: Using a different version of >> jlink than the java.base you're linking will lose some optimizations and the >> user would be none the wiser as to why, verbosity helps avoid surprises. > > The plugin is enabled by default. With this change, I consider > this plugin is "auto-enabled" when it’s creating the image of > the version that this plugin supports (i.e. matching major.minor > version). > > So if the —generate-jli-classes option is not specified, it might > be confusing when I get this warning. I would prefer in this case > no warning should be emitted and the plugin is not enabled. > > If the option is specified on the command-line, it should emit > the warning. > >> >> http://cr.openjdk.java.net/~redestad/8175026/jdk.02/ > > 322 if (!initialize(in)) { > > Maybe refactor line 175-190 in a new method and something like this: > if (!checkVersion(getLinkedVersion(in))) > : > } > > Then follow with initialize(in) here. That’d make it explicit. > One thing to handle is when exception is thrown when reading > the trace file (default or mainArgument). Maybe that part can > be done early in configure method and store the lines for later > consumption. > > line 235-238: you may use orThrow in this case. > > Mandy
