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.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/%**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
