Hi Sean,

Patch pushed @ http://hg.openjdk.java.net/jdk8/tl/jdk/rev/83765e82cacb
Could you please verify?

Thanks & regards
Jonathan

On 11/14/2012 10:23 AM, Sean Chou wrote:
Thanks Alan and Chris.


On Tue, Nov 13, 2012 at 7:21 PM, Chris Hegarty <chris.hega...@oracle.com>wrote:

Sean,

Looks good to me. Thanks for updating the test.

-Chris.


On 11/13/2012 03:17 AM, Sean Chou wrote:

Hi Alan,

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


On Mon, Nov 12, 2012 at 6:00 PM, Alan Bateman <alan.bate...@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/~**zhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/>
<
http://cr.openjdk.java.net/%****7Ezhouyx/7201156/webrev.02/<ht**
tp://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.







Reply via email to