Hi,
Updated to include location (when available) and formatting:
http://cr.openjdk.java.net/~sundar/8189671/webrev.02/
-Sundar
On 20/10/17, 3:37 AM, mandy chung wrote:
Hi Sunder,
Updated: http://cr.openjdk.java.net/~sundar/8189671/webrev.01/
This version checks the resolved modules which is good. One other
comment:
444 throw new
IllegalArgumentException(taskHelper.getMessage("err.automatic.module",
modDesc.name()));It may be useful to include the pathname to the
exception message. Nit: long line and good to wrap it into multiple
lines.
Mandy
P.S. My previous reply lost the formatting and I fixed the copy below
(just as a record for easy viewing).
On 10/19/17 2:58 PM, mandy chung wrote:
On 10/19/17 7:04 AM, Sundararajan Athijegannathan wrote:
Please review.
Bug: https://bugs.openjdk.java.net/browse/JDK-8189671
Webrev: http://cr.openjdk.java.net/~sundar/8189671/webrev.00/
441 ModuleFinder finder = config.finder();
442 for (String root : config.getModules()) {
This should check the resolved modules rather than just the root
modules since a resolved module may be an automatic module but not
a root. You may want to add such a test case too.
(A side note: JlinkConfiguration::getModules perhaps should be
renamed to `roots` since it returns the root modules specified
in jlink --add-modules)
447 throw new
IllegalArgumentException(taskHelper.getMessage("err.automatic.module.as.root",
modDesc.name()));
It may be useful to include the pathname to the exception message.
Nit: long line and good to wrap it into multiple lines.
Mandy
