here is the webrevs w/ JarUtils from default package inserted into 
jdk.test.lib.util.JarUtils:
whole patch: 
http://cr.openjdk.java.net/~iignatyev//8211171/webrev.01/index.html 
<http://cr.openjdk.java.net/~iignatyev//8211171/webrev.01/index.html>
> 655 lines changed: 239 ins; 355 del; 61 mod;
incremental: 
http://cr.openjdk.java.net/~iignatyev//8211171/webrev.0-1/index.html 
<http://cr.openjdk.java.net/~iignatyev//8211171/webrev.0-1/index.html>
> 476 lines changed: 239 ins; 203 del; 34 mod;

doing that, I noticed that both updateJarFile and createJarFile don't close 
Stream<Path> from Files::find, the current patch fixes that.

-- Igor

> On Sep 26, 2018, at 2:25 PM, Igor Ignatyev <igor.ignat...@oracle.com> wrote:
> 
> Hi Alan,
> 
> it appears that I have to put into a named package as the jaxp test which 
> uses it is in a named package.
> 
> although I don't want to group simple moving and more sophisticated test 
> conversion for the sake of reviewers, also there is another "duplicate" of 
> JarUtils in test/jdk/lib/testlibrary/java/util/jar/JarBuilder.java. I'd 
> prefer to "merge" all three instances together, where merge might be 
> implemented as conversion of tests.
> 
> so as an intermediate step, I propose to move all the methods of JarUtils 
> into jdk.test.lib.util.JarUtils.
> 
> Thanks,
> -- Igor
>> On Sep 26, 2018, at 1:11 PM, Alan Bateman <alan.bate...@oracle.com> wrote:
>> 
>> 
>> 
>> On 26/09/2018 19:16, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8211171/webrev.00/index.html
>> Looks good but maybe it should be put into a named package (see below).
>> 
>>> :
>>> please note, there is another version of JarUtils in jdk.test.lib.util 
>>> package, but these two versions have different signatures and behavior, 
>>> merging them is a bit tricker and will be done separately.
>>> 
>> The JarUtils that are moving today to test/lib is one that I added as test 
>> infrastructure some time ago. I don't where 
>> test/lib/jdk/test/lib/util/JarUtils.jav came from but it seems less useful. 
>> It makes me wonder if it would be better to just convert tests rather than 
>> merging. A few convenience methods added to the JarUtils you are moving 
>> would make that easier.
>> 
>> -Alan
> 

Reply via email to