RFR: jmod remove

Mandy Chung mandy.chung at oracle.com
Thu May 10 14:59:31 PDT 2012


Chris,

Looks good in general.  Some minor comments below:

On 5/9/2012 7:31 AM, Chris Hegarty wrote:
> http://cr.openjdk.java.net/~chegar/jigsaw/jmodrm.06/webrev/

Files.java
   You can also remove L39.
   L147: Could you just add exc to the "excs" list rather
   than wrapping it with a new IOException?

Librarian.java
   L317: The "excs" variable is not needed any more.
   L327: Should warnings normally go to System.err instead of
   System.out?

SimpleLibrary.java
   L1505: should the remove method accept a declaring ModuleId
   as a valid argument?  I am wondering whether it should allow
   to remove a module if an alias name or non-default view name
   is given.  Perhaps add a comment and leave it as an open issue
   to be addressed later.

   L1546-1548:  Another way to perform this check would be:
      for (ModuleId mid : mids) {
         if (cf.findContextForModuleName(mid) != null)
            throw new ....
      }
   Would that be more explicit?

ModuleFile.java
   L525: should be aligned 2-space to the left

Mandy
P.S.  I hope to send you my comments this morning so that you
can rev it once.  I was almost done before going to 11am and
got tied up until now.  Sorry for causing you to generate
another round of webrev.




More information about the jigsaw-dev mailing list