RFR(S) : 8180724: move ModuleInfoMaker to the top level testlibrary

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(S) : 8180724: move ModuleInfoMaker to the top level testlibrary

Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8180724/webrev.00/index.html
> 295 lines changed: 139 ins; 140 del; 16 mod;

Hi all,

could you please review this small fix which moves ModuleInfoMaker class from jdk testlibrary to the top level testlibrary? since ModuleInfoMaker depends on CompilerUtil and thus on jdk.compiler module, it has been placed to jdk.test.lib.compiler package to avoid unneeded module dependency in tests. HashesTest has been updated to use ModuleInfoMaker::compile instead of using CompilerUtils::compile directly.

webrev: http://cr.openjdk.java.net/~iignatyev//8180724/webrev.00/index.html
jbs: https://bugs.openjdk.java.net/browse/JDK-8180724
testing: affected tests (jdk/test/tools/launcher)

Thanks,
-- Igor
Reply | Threaded
Open this post in threaded view
|

Re: RFR(S) : 8180724: move ModuleInfoMaker to the top level testlibrary

Paul Sandoz
+1
Paul.

> On 19 May 2017, at 16:29, Igor Ignatyev <[hidden email]> wrote:
>
> http://cr.openjdk.java.net/~iignatyev//8180724/webrev.00/index.html
>> 295 lines changed: 139 ins; 140 del; 16 mod;
>
> Hi all,
>
> could you please review this small fix which moves ModuleInfoMaker class from jdk testlibrary to the top level testlibrary? since ModuleInfoMaker depends on CompilerUtil and thus on jdk.compiler module, it has been placed to jdk.test.lib.compiler package to avoid unneeded module dependency in tests. HashesTest has been updated to use ModuleInfoMaker::compile instead of using CompilerUtils::compile directly.
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8180724/webrev.00/index.html
> jbs: https://bugs.openjdk.java.net/browse/JDK-8180724
> testing: affected tests (jdk/test/tools/launcher)
>
> Thanks,
> -- Igor

Reply | Threaded
Open this post in threaded view
|

Re: RFR(S) : 8180724: move ModuleInfoMaker to the top level testlibrary

Mandy Chung
In reply to this post by Igor Ignatyev

> On May 19, 2017, at 4:29 PM, Igor Ignatyev <[hidden email]> wrote:
>
> could you please review this small fix which moves ModuleInfoMaker class from jdk testlibrary to the top level testlibrary? since ModuleInfoMaker depends on CompilerUtil and thus on jdk.compiler module, it has been placed to jdk.test.lib.compiler package to avoid unneeded module dependency in tests. HashesTest has been updated to use ModuleInfoMaker::compile instead of using CompilerUtils::compile directly.
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8180724/webrev.00/index.html
> jbs: https://bugs.openjdk.java.net/browse/JDK-8180724
> testing: affected tests (jdk/test/tools/launcher)

jdk/test/tools/jmod/hashes/HashesTest.java
  Nit: can you keep line 307-309, 386-387 unchanged?

Otherwise, looks fine.
Mandy

Reply | Threaded
Open this post in threaded view
|

Re: RFR(S) : 8180724: move ModuleInfoMaker to the top level testlibrary

Igor Ignatyev
sure, if you prefer that style more, I'll undo these changes before pushing.
thanks for review.

-- Igor

> On May 24, 2017, at 2:19 PM, Mandy Chung <[hidden email]> wrote:
>
>
>> On May 19, 2017, at 4:29 PM, Igor Ignatyev <[hidden email]> wrote:
>>
>> could you please review this small fix which moves ModuleInfoMaker class from jdk testlibrary to the top level testlibrary? since ModuleInfoMaker depends on CompilerUtil and thus on jdk.compiler module, it has been placed to jdk.test.lib.compiler package to avoid unneeded module dependency in tests. HashesTest has been updated to use ModuleInfoMaker::compile instead of using CompilerUtils::compile directly.
>>
>> webrev: http://cr.openjdk.java.net/~iignatyev//8180724/webrev.00/index.html
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8180724
>> testing: affected tests (jdk/test/tools/launcher)
>
> jdk/test/tools/jmod/hashes/HashesTest.java
>  Nit: can you keep line 307-309, 386-387 unchanged?
>
> Otherwise, looks fine.
> Mandy
>