Request for review: 7201156 : jar tool fails to convert file separation characters for list and extract

[Sean Chou]

Hi Alan,

Here is the updated webrev:
http://cr.openjdk.java.net/~zhouyx/7201156/webrev.03/  .


On Mon, Nov 12, 2012 at 6:00 PM, Alan Bateman <Alan.Bateman at oracle.com>wrote:

> On 12/11/2012 09:08, Sean Chou wrote:
>
>> Hi ,
>>
>> I didn't realize that rt.jar would miss before. The testcase is updated
>> to create a temp file as well as test the extract option. However, extract
>> testing will create a directory if it passes, I just deleted it in
>> testcase.  Please take a look.
>>
>> webrev: http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/><
>> http://cr.openjdk.java.net/%**7Ezhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/>>
>>  .
>>
>>  Thanks for removing the dependency on rt.jar.
>
> A couple of comments on the updated test:
>
> - in main then it still has "pathRtJar" so I assume you just forgot to
> rename that.
>
I forgot the name had its meaning.

>
> - it would be good to change createJarFile to use try-with-resources so
> that an error doesn't cause it to terminate with an open file.
>
Changed.

>
> - testJarExact, I assume you intended to name this testJarExtract.
>
Yes!

>
> - L114-117, the "./" is not needed. It would be okay to leave those files
> there if you want, jtreg will clean them up too.

Thanks for the hint. I removed these lines, so the testcase looks tidy.

>
>
> -Alan.
>
>
>


-- 
Best Regards,
Sean Chou