Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

> On Nov 23, 2015, at 4:40 PM, Alan Bateman wrote: > > > > On 23/11/2015 15:27, Attila Szegedi wrote: >> Folks, >> >> I integrated the changes Mandy suggested; please review these (build >> related) changes: >> jdk9 top level: >>

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

But, in addition to providing service, jdk.scripting.nashorn module also "exports" nashorn specific APIs in jdk.nashorn.api.* packages. -Sundar On 11/23/2015 9:10 PM, Alan Bateman wrote: On 23/11/2015 15:27, Attila Szegedi wrote: Folks, I integrated the changes Mandy suggested; please

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

On 23/11/2015 16:07, Attila Szegedi wrote: : Whichever is the stronger criteria for deciding whether to place it in MAIN or PROVIDER is fine with me. Intuitively “provider” seems like a weaker category (exposes a service provider but doesn’t have its own API), so (without knowing the

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

On 23/11/2015 15:43, Sundararajan Athijegannathan wrote: But, in addition to providing service, jdk.scripting.nashorn module also "exports" nashorn specific APIs in jdk.nashorn.api.* packages. Sure, it could go in either but we mostly treat it as a service provider. -Alan.

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

> On Nov 23, 2015, at 7:40 AM, Alan Bateman wrote: > > > > On 23/11/2015 15:27, Attila Szegedi wrote: >> Folks, >> >> I integrated the changes Mandy suggested; please review these (build >> related) changes: >> jdk9 top level: >>

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

> On Nov 23, 2015, at 8:42 AM, Alan Bateman wrote: > > We need to do a bit of clean-up in Images.gmk to make things clearer as this > MAIN vs. PROVIDER topic has caused confusion on a few cases. If we can keep > the lists separate to the list of modules for the

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

+1 for the Nashorn/Dynalink changes. The top level changes look good to me but I'd prefer to leave reviews to those who are more familiar with the build/modules infrastructure. Hannes Am 2015-11-20 um 00:15 schrieb Attila Szegedi: Please review JDK-8141338 "Move jdk.internal.dynalink

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

Folks, I integrated the changes Mandy suggested; please review these (build related) changes: jdk9 top level: jdk9/jdk: For the sake of

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

On 23/11/2015 15:27, Attila Szegedi wrote: Folks, I integrated the changes Mandy suggested; please review these (build related) changes: jdk9 top level: jdk9/jdk:

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

Thanks for pointing out these, Mandy; I will make the changes you suggested. Attila. > On Nov 20, 2015, at 6:10 AM, Mandy Chung wrote: > > jdk.scripting.nashorn is loaded by the extension class loader. Is > jdk.dynalink expected to be loaded by the ext. class loader?

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

On Nov 20, 2015, at 4:10 PM, Alan Bateman wrote: > > On 19/11/2015 23:15, Attila Szegedi wrote: >> Please review JDK-8141338 "Move jdk.internal.dynalink package to >> jdk.dynalink" for . This >> is basically the

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

On 19/11/2015 23:15, Attila Szegedi wrote: Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" for . This is basically the implementation step for integrating JEP 276. This changeset will introduce a new public API

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

jdk.scripting.nashorn is loaded by the extension class loader. Is jdk.dynalink expected to be loaded by the ext. class loader? You need to edit this file to include the new module: jdk/make/src/classes/build/tools/module/ext.modules This is an interim file to map modules to class loader and

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

I reviewed the top repo change. modules.xml looks fine. jdk.dynalink should be in MAIN_MODULES since it has exported APIs. jdk.scripting.nashorn should be moved too. They are not sole service providers. Since you are on this file, can you move jdk.scripting.nashorn to MAIN_MODULES as

Re: Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

+1 on all changes. -Sundar On 11/20/2015 12:15 AM, Attila Szegedi wrote: Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" for . This is basically the implementation step for integrating JEP 276. This changeset