> On Oct 24, 2016, at 10:28 AM, Steve Drach <steve.dr...@oracle.com> wrote:
> 
> There is a new webrev at 
> http://cr.openjdk.java.net/~sdrach/8164805/webrev.01/ 

sun/tools/jar/Main.java

Thanks for refactoring and adding the findConcealedPackages method.  What I 
actually meant was to move out this line:
    concealedPackages = findConcealedPackages(rd);

to probably before calling addExtendedModuleAttributes(moduleInfos) above line 
342 and 1101.

2014                 .filter(p -> !p.equals("”))

For a modular JAR, there should be no unnamed package.  I think the jar tool 
should fail if it detects an unnamed package.  Your test does not have any 
unnamed package - how did you find this?

ConcealedPackage.java test

Thanks for improving the test.  It’d be good to name the @Test method with a 
descriptive method name e.g. 
   test1 -> testUpdateVersionedPublicClass
   test2 -> testUpdatedVersionedPublicConcealedClass

 117     @Test // updates a valid multi-release jar with a new public class in
 118           // versioned section and fails

Nit: You can consider moving the comment above @Test.

Mandy


Reply via email to