Please review: 7125442

2012-01-10 Thread Kumar Srinivasan

Hi,

Please review:
CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7125442

Webrev:
http://cr.openjdk.java.net/~ksrini/7124443/

Thanks
Kumar



Re: Please review: 7125442

2012-01-10 Thread Mandy Chung

I18NJarTest.java
   L29: Is -XDignore.symbol.file really needed?  As far as I see,
   the test only uses the public APIs.
   L79: should it check if the returned value is false?

Otherwise, looks good.

Mandy

On 1/10/2012 1:06 PM, Kumar Srinivasan wrote:

sorry I pasted the wrong webrev in the email,
here is the right one...
http://cr.openjdk.java.net/~ksrini/7125442/

Kumar




Hi,

Please review:
CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7125442

Webrev:
http://cr.openjdk.java.net/~ksrini/7124443/

Thanks
Kumar





Re: Please review: 7125442

2012-01-10 Thread Kumar Srinivasan



I18NJarTest.java
   L29: Is -XDignore.symbol.file really needed?  As far as I see,
   the test only uses the public APIs.


This is needed,  since TestHelper uses javac and tar  apis
behind the scenes.


   L79: should it check if the returned value is false?


IMO not necessary, if mkdir fails then createJar will throw exception.




Otherwise, looks good.


Thanks
Kumar



Mandy

On 1/10/2012 1:06 PM, Kumar Srinivasan wrote:

sorry I pasted the wrong webrev in the email,
here is the right one...
http://cr.openjdk.java.net/~ksrini/7125442/

Kumar




Hi,

Please review:
CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7125442

Webrev:
http://cr.openjdk.java.net/~ksrini/7124443/

Thanks
Kumar







Re: Please review: 7125442

2012-01-10 Thread Mandy Chung

On 1/10/2012 1:44 PM, Kumar Srinivasan wrote:



I18NJarTest.java
   L29: Is -XDignore.symbol.file really needed?  As far as I see,
   the test only uses the public APIs.


This is needed,  since TestHelper uses javac and tar  apis
behind the scenes.


Ah.  I missed its reference to sun.tools.jar.Main.  It uses
javax.tools.ToolProvider and JavaCompiler API and so sun.tools.jar
api is the only one.




   L79: should it check if the returned value is false?


IMO not necessary, if mkdir fails then createJar will throw exception.



Okay.  Thanks.

Mandy


Re: Please review: 7125442

2012-01-10 Thread Jeannette Hung
Kumar,
   Could you fill in the introduced-in field in the CR?  

Thanks
jeannette

On Jan 10, 2012, at 1:52 PM, Mandy Chung wrote:

 On 1/10/2012 1:44 PM, Kumar Srinivasan wrote:
 
 I18NJarTest.java
   L29: Is -XDignore.symbol.file really needed?  As far as I see,
   the test only uses the public APIs.
 
 This is needed,  since TestHelper uses javac and tar  apis
 behind the scenes.
 
 Ah.  I missed its reference to sun.tools.jar.Main.  It uses
 javax.tools.ToolProvider and JavaCompiler API and so sun.tools.jar
 api is the only one.
 
 
   L79: should it check if the returned value is false?
 
 IMO not necessary, if mkdir fails then createJar will throw exception.
 
 
 Okay.  Thanks.
 
 Mandy