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 <[email protected]> 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 <[email protected]> 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 >>>>>> >>>> >>> >> >
