RFR of JDK-8186610,move ModuleUtils to top-level testlibrary

Igor Ignatyev igor.ignatyev at oracle.com
Thu Oct 11 18:51:32 UTC 2018


Hi Hamlin,

the key point here, ModuleTargetHelper.java is not a "library" class, it's a part of tools/jlink/plugins/SystemModuleDescriptors/ tests.

thanks for updating webrev, I have one nit --- given ModuleUtils is the only class in jdk/test/lib/module package, I doubt we need to introduce this package, ModuleUtils can be put into j.t.l.util package.

-- Igor.

> On Oct 10, 2018, at 11:31 PM, Hamlin Li <huaming.li at oracle.com> wrote:
> 
> Thank you clarifying Igor.
> 
> Moving ModuleTargetHelper to local folder has a drawback: it's hard for future maintainer to found it if they need to use it in other places, that make it an "invisible" library class.
> Although I don't fully agree with you, I have updated the webrev as you suggested: http://cr.openjdk.java.net/~mli/8186610/webrev.01/ <http://cr.openjdk.java.net/~mli/8186610/webrev.01/>
> Thank you
> 
> -Hamlin
> 
> On 2018/10/11 11:38 AM, Igor Ignatyev wrote:
>> b/c this will make test library modularization[1] somewhat troublesome, also I ain't sure if ModuleTargetHelper really needs to be placed into the top-level library regardless of its dependency on non-public API. "promoting" test library class to the top-level library comes w/ increased maintenance costs, the parent task[2] explains that in more details. 
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8211358 <https://bugs.openjdk.java.net/browse/JDK-8211358>
>> [2] https://bugs.openjdk.java.net/browse/JDK-8211290 <https://bugs.openjdk.java.net/browse/JDK-8211290>
>> 
>> -- Igor
>> 
>>> On Oct 10, 2018, at 8:26 PM, Hamlin Li <huaming.li at oracle.com <mailto:huaming.li at oracle.com>> wrote:
>>> 
>>> Hi Igor,
>>> 
>>> Would you please clarify your concern further? I mean why ModuleTargetHelper can be put to top level when it use non-public APIs e.g. jdk.internal.module.*?
>>> 
>>> Thank you
>>> 
>>> -Hamlin
>>> 
>>> On 2018/10/11 11:08 AM, Igor Ignatyev wrote:
>>>> Hi Hamlin,
>>>> 
>>>> as ModuleTargetHelper uses non-public API, I'd prefer not to have in a common test library, and 8211976 suggests moving it closer to tests. could you please explain why you decided to put it into the library instead?
>>>> 
>>>> Thanks,
>>>> -- Igor
>>>> 
>>>> ----- Original Message -----
>>>> From: huaming.li at oracle.com <mailto:huaming.li at oracle.com>
>>>> To: core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>
>>>> Sent: Wednesday, October 10, 2018 7:40:34 PM GMT -08:00 US/Canada Pacific
>>>> Subject: RFR of JDK-8186610,move ModuleUtils to top-level testlibrary
>>>> 
>>>> Would you please review the following patch?
>>>> 
>>>> bug:
>>>> 
>>>>    https://bugs.openjdk.java.net/browse/JDK-8186610 <https://bugs.openjdk.java.net/browse/JDK-8186610>
>>>> 
>>>>    https://bugs.openjdk.java.net/browse/JDK-8211976 <https://bugs.openjdk.java.net/browse/JDK-8211976>
>>>> 
>>>> webrev: http://cr.openjdk.java.net/~mli/8186610/ <http://cr.openjdk.java.net/%7Emli/8186610/>
>>>> 
>>>> 
>>>> Thank you
>>>> 
>>>> -Hamlin
>>>> 
>>> 
>> 
> 



More information about the core-libs-dev mailing list