Review Request: JDK-8168836 Minor clean up on warning/error messages on --add-exports and --add-reads

Mandy Chung mandy.chung at oracle.com
Mon Dec 19 19:39:32 UTC 2016


> On Dec 19, 2016, at 9:45 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> 
> 
> On 19/12/2016 01:44, Mandy Chung wrote:
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8168836/webrev.00/
>> 
>> This patch improves the warning/error message to include the option name, emit a warning if unknown module is specified with —-patch-module be consistent with the options.
>> 
> The implementation update looks good.  As --patch-modules is a slow path then you could use cf.findModule(mn).ifPresent(…)

Fixed.

> ExplodedModuleBuilder is useful infrastructure but I'm not sure that test/tools/lib is the right place as that location seems to be for jimage infrastructure (I've often thought this should move). Maybe test/lib/testlibrary would be better because that there is where ModuleUtils and the other test infrastructure used by these tests is. Also just wondering if there is a better name for this class as it supports both creation and compilation, maybe it should be in ModuleUtils as Builder or ModuleMaker (you might have tried a few names already). Once this is in then we might look at changes some of the existing tests to use it.

What about renaming it to test/lib/testlibrary/ModuleSourceBuilder.java?

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8168836/webrev.01

Mandy


More information about the jigsaw-dev mailing list