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/

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
[2] https://bugs.openjdk.java.net/browse/JDK-8211290

-- Igor

On Oct 10, 2018, at 8:26 PM, Hamlin Li <huaming...@oracle.com <mailto:huaming...@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...@oracle.com <mailto:huaming...@oracle.com>
To: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@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-8211976

webrev: http://cr.openjdk.java.net/~mli/8186610/ <http://cr.openjdk.java.net/%7Emli/8186610/>


Thank you

-Hamlin




Reply via email to