Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
Hi Hamlin, Although I understand your reasoning, I do share Amy's concerns on doing less than the RFEs ask for (as w/ 8211972). so I'd suggest you to split your patch into 4 separate patches and RFRs, one per original RFE. this won't just return sanity to reviewers and review, reduce chance of leaving already fixed bugs forgotten/forsaken, but will also make it possible to push ones which don't cause any concerns. Thanks, -- Igor > On Oct 11, 2018, at 11:28 PM, Amy Lu wrote: > > On 2018/10/12 2:16 PM, Hamlin Li wrote: >> yes, e.g. https://bugs.openjdk.java.net/browse/JDK-8212033 > > It seems mentioned bug is duplicate with > > JDK-8211972: remove testlibrary/java/util/jar/Compiler.java - "suggest > removing and using jdk.test.lib.compiler.InMemoryJavaCompiler instead" > > which is included in this changeset. > > Thanks, > Amy > >> >> Thank you >> >> -Hamlin >> >> >> On 2018/10/12 2:13 PM, Amy Lu wrote: >>> Hi, Hamlin >>> >>> - test/lib/jdk/test/lib/compiler/Compiler.java (was >>> test/jdk/lib/testlibrary/java/util/jar/Compiler.java) >>> Any future plan to "merge" it with existing >>> jdk.test.lib.compiler.CompilerUtils? >>> >>> - test/lib/jdk/test/lib/util/JarBuilder.java (was >>> test/jdk/lib/testlibrary/java/util/jar/JarBuilder.java) >>> Any future plan to "merge" it with existing jdk.test.lib.util.JarUtils? >>> >>> Thanks, >>> Amy >>> >>> On 2018/10/12 2:00 PM, Hamlin Li wrote: Hi Igor, It's updated in place http://cr.openjdk.java.net/~mli/8211974/webrev.00/, please review it again. Thank you -Hamlin On 2018/10/12 1:34 PM, Igor Ignatyev wrote: > Hi Hamlin, > > could you please move jdk.test.lib.util.Compiler to j.t.l.compiler > package? we use this package for classes which have dependency on > jdk.compiler and/or java.compiler module. > > it'd also be nice to put CreateMultiReleaseTestJars into a named package. > > -- Igor >> On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: >> >> would you please review the following patch? >> >> bug: >> >>https://bugs.openjdk.java.net/browse/JDK-8211974 >> >>https://bugs.openjdk.java.net/browse/JDK-8211972 >> >>https://bugs.openjdk.java.net/browse/JDK-8211973 >> >>https://bugs.openjdk.java.net/browse/JDK-8211979 >> >> webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ >> >> Thank you >> >> -Hamlin >> >>> >> >
Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
Ping... On 2018/10/12 2:00 PM, Hamlin Li wrote: Hi Igor, It's updated in place http://cr.openjdk.java.net/~mli/8211974/webrev.00/, please review it again. Thank you -Hamlin On 2018/10/12 1:34 PM, Igor Ignatyev wrote: Hi Hamlin, could you please move jdk.test.lib.util.Compiler to j.t.l.compiler package? we use this package for classes which have dependency on jdk.compiler and/or java.compiler module. it'd also be nice to put CreateMultiReleaseTestJars into a named package. -- Igor On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: would you please review the following patch? bug: https://bugs.openjdk.java.net/browse/JDK-8211974 https://bugs.openjdk.java.net/browse/JDK-8211972 https://bugs.openjdk.java.net/browse/JDK-8211973 https://bugs.openjdk.java.net/browse/JDK-8211979 webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ Thank you -Hamlin
Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
On 2018/10/12 2:16 PM, Hamlin Li wrote: yes, e.g. https://bugs.openjdk.java.net/browse/JDK-8212033 It seems mentioned bug is duplicate with JDK-8211972: remove testlibrary/java/util/jar/Compiler.java - "suggest removing and using jdk.test.lib.compiler.InMemoryJavaCompiler instead" which is included in this changeset. Thanks, Amy Thank you -Hamlin On 2018/10/12 2:13 PM, Amy Lu wrote: Hi, Hamlin - test/lib/jdk/test/lib/compiler/Compiler.java (was test/jdk/lib/testlibrary/java/util/jar/Compiler.java) Any future plan to "merge" it with existing jdk.test.lib.compiler.CompilerUtils? - test/lib/jdk/test/lib/util/JarBuilder.java (was test/jdk/lib/testlibrary/java/util/jar/JarBuilder.java) Any future plan to "merge" it with existing jdk.test.lib.util.JarUtils? Thanks, Amy On 2018/10/12 2:00 PM, Hamlin Li wrote: Hi Igor, It's updated in place http://cr.openjdk.java.net/~mli/8211974/webrev.00/, please review it again. Thank you -Hamlin On 2018/10/12 1:34 PM, Igor Ignatyev wrote: Hi Hamlin, could you please move jdk.test.lib.util.Compiler to j.t.l.compiler package? we use this package for classes which have dependency on jdk.compiler and/or java.compiler module. it'd also be nice to put CreateMultiReleaseTestJars into a named package. -- Igor On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: would you please review the following patch? bug: https://bugs.openjdk.java.net/browse/JDK-8211974 https://bugs.openjdk.java.net/browse/JDK-8211972 https://bugs.openjdk.java.net/browse/JDK-8211973 https://bugs.openjdk.java.net/browse/JDK-8211979 webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ Thank you -Hamlin
Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
yes, e.g. https://bugs.openjdk.java.net/browse/JDK-8212033 Thank you -Hamlin On 2018/10/12 2:13 PM, Amy Lu wrote: Hi, Hamlin - test/lib/jdk/test/lib/compiler/Compiler.java (was test/jdk/lib/testlibrary/java/util/jar/Compiler.java) Any future plan to "merge" it with existing jdk.test.lib.compiler.CompilerUtils? - test/lib/jdk/test/lib/util/JarBuilder.java (was test/jdk/lib/testlibrary/java/util/jar/JarBuilder.java) Any future plan to "merge" it with existing jdk.test.lib.util.JarUtils? Thanks, Amy On 2018/10/12 2:00 PM, Hamlin Li wrote: Hi Igor, It's updated in place http://cr.openjdk.java.net/~mli/8211974/webrev.00/, please review it again. Thank you -Hamlin On 2018/10/12 1:34 PM, Igor Ignatyev wrote: Hi Hamlin, could you please move jdk.test.lib.util.Compiler to j.t.l.compiler package? we use this package for classes which have dependency on jdk.compiler and/or java.compiler module. it'd also be nice to put CreateMultiReleaseTestJars into a named package. -- Igor On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: would you please review the following patch? bug: https://bugs.openjdk.java.net/browse/JDK-8211974 https://bugs.openjdk.java.net/browse/JDK-8211972 https://bugs.openjdk.java.net/browse/JDK-8211973 https://bugs.openjdk.java.net/browse/JDK-8211979 webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ Thank you -Hamlin
Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
Hi, Hamlin - test/lib/jdk/test/lib/compiler/Compiler.java (was test/jdk/lib/testlibrary/java/util/jar/Compiler.java) Any future plan to "merge" it with existing jdk.test.lib.compiler.CompilerUtils? - test/lib/jdk/test/lib/util/JarBuilder.java (was test/jdk/lib/testlibrary/java/util/jar/JarBuilder.java) Any future plan to "merge" it with existing jdk.test.lib.util.JarUtils? Thanks, Amy On 2018/10/12 2:00 PM, Hamlin Li wrote: Hi Igor, It's updated in place http://cr.openjdk.java.net/~mli/8211974/webrev.00/, please review it again. Thank you -Hamlin On 2018/10/12 1:34 PM, Igor Ignatyev wrote: Hi Hamlin, could you please move jdk.test.lib.util.Compiler to j.t.l.compiler package? we use this package for classes which have dependency on jdk.compiler and/or java.compiler module. it'd also be nice to put CreateMultiReleaseTestJars into a named package. -- Igor On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: would you please review the following patch? bug: https://bugs.openjdk.java.net/browse/JDK-8211974 https://bugs.openjdk.java.net/browse/JDK-8211972 https://bugs.openjdk.java.net/browse/JDK-8211973 https://bugs.openjdk.java.net/browse/JDK-8211979 webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ Thank you -Hamlin
Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
Hi Igor, It's updated in place http://cr.openjdk.java.net/~mli/8211974/webrev.00/, please review it again. Thank you -Hamlin On 2018/10/12 1:34 PM, Igor Ignatyev wrote: Hi Hamlin, could you please move jdk.test.lib.util.Compiler to j.t.l.compiler package? we use this package for classes which have dependency on jdk.compiler and/or java.compiler module. it'd also be nice to put CreateMultiReleaseTestJars into a named package. -- Igor On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: would you please review the following patch? bug: https://bugs.openjdk.java.net/browse/JDK-8211974 https://bugs.openjdk.java.net/browse/JDK-8211972 https://bugs.openjdk.java.net/browse/JDK-8211973 https://bugs.openjdk.java.net/browse/JDK-8211979 webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ Thank you -Hamlin
Re: RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
Hi Hamlin, could you please move jdk.test.lib.util.Compiler to j.t.l.compiler package? we use this package for classes which have dependency on jdk.compiler and/or java.compiler module. it'd also be nice to put CreateMultiReleaseTestJars into a named package. -- Igor > On Oct 11, 2018, at 10:23 PM, Hamlin Li wrote: > > would you please review the following patch? > > bug: > > https://bugs.openjdk.java.net/browse/JDK-8211974 > > https://bugs.openjdk.java.net/browse/JDK-8211972 > > https://bugs.openjdk.java.net/browse/JDK-8211973 > > https://bugs.openjdk.java.net/browse/JDK-8211979 > > webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ > > Thank you > > -Hamlin >
RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary
would you please review the following patch? bug: https://bugs.openjdk.java.net/browse/JDK-8211974 https://bugs.openjdk.java.net/browse/JDK-8211972 https://bugs.openjdk.java.net/browse/JDK-8211973 https://bugs.openjdk.java.net/browse/JDK-8211979 webrev: http://cr.openjdk.java.net/~mli/8211974/webrev.00/ Thank you -Hamlin
