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