On 19/11/2015 23:15, Attila Szegedi wrote:
Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" for 
<https://bugs.openjdk.java.net/browse/JDK-8141338>. This is basically the implementation 
step for integrating JEP 276. This changeset will introduce a new public API that has CCC 
approval (request 8075866), and is also the implementation step of JEP 276 which is now 
targeted for 9 and thus can be integrated.

The changes in this changeset fall into several categories:
- renaming of jdk.internal.dynalink.* package to jdk.dynalink.* package, with 
ripple effects in Nashorn classes that import from these packages
- changes to modules.xml and some build files to accommodate a new public 
module and a dependency of Nashorn on it
- new tests

  I’m sending this webrev to several lists
Probably build-dev instead of jdk9-dev.

I'm curious if it's strictly necessary for module jdk.dynalink to be in the nashorn repo now, I assume not but it's probably convenient when working Nashorn.

In any case, the module name and the changes to modules.xml look okay to me. As Mandy noted, this isn't a service provider API so in Images.gmk then you can add it to MAIN_MODULES rather than PROVIDER_MODULES.

Your webrevs don't have the changes to the jdk repo but I assume that make/src/classes/build/tools/module/ext.modules has been updated to list jdk.dynalink. That is, I assume it needs to be defined to the ext loader because jdk.scripting.nashorn uses it. The ext.modules file is temporary and goes away when we bring in the module system.

-Alan.


Reply via email to