Review request on the optional modules support
Alan Bateman
Alan.Bateman at oracle.com
Mon Aug 8 08:01:16 PDT 2011
Mandy Chung wrote:
> An update on the optional module support [1].
>
> Webrev:
> http://cr.openjdk.java.net/~mchung/jigsaw/webrevs/optional-modules.00/
>
> The note has been updated to include the open questions raised in
> the discussion.
> http://openjdk.java.net/projects/jigsaw/doc/topics/optional.html
I went through the webrev.
It mostly looks okay to me but I suspect that developers might initially
grapple with isModulePresent and requireModulePresent as instance
methods (in time I guess the javadoc will be fleshed out and concepts
such as context can be referenced).
I see you've changed a few of the SecurityManager methods to throw
ModuleNotPresentException but I'm not sure that is really needed. The
alternative is to just have the permission checks fail, as they do now,
when the desktop module is not present.
The updates to java.lang.reflect.Module remind me to ask whether this is
the right package for this class (java.lang, java.lang.reflect or
java.lang.module). I realize there is history here (should Class and
Package be in java.lang.reflect, etc.) but it does seem inconsistent now.
One of the questions posed is whether the compiler can use the
RequireOptionalModule annotations but I would have thought that if there
are any references to types in optional modules then it will require
that they be present at compile time. Maybe this question is only for
the case that reflection is used?
Another question is about handling different versions but that strikes
me as a bit fragile and could potentially get out of sync with the
version constraints in the module-info too.
Otherwise I think the changes are okay and I note that
RequireOptionalModule will need to have to classes element removed once
we don't require the class analyzer in the build.
-Alan
More information about the jigsaw-dev
mailing list