RFR of JDK-8211974,move testlibrary/java/util/jar/CreateMultiReleaseTestJars.java to a separate testlibrary

Igor Ignatyev igor.ignatyev at oracle.com
Thu Nov 15 01:36:26 UTC 2018


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 <amy.lu at oracle.com> 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 <huaming.li at oracle.com> 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
>>>>>> 
>>>> 
>>> 
>> 
> 



More information about the core-libs-dev mailing list