> On Jan 31, 2017, at 10:26 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
> 
> Hi,
> 
> Please find below a patch for:
> 
> 8173607: JMX RMI connector should be in its own module
> https://bugs.openjdk.java.net/browse/JDK-8173607
> 
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8173607/webrev.05
> 

This mostly looks good.  I’d like to see an updated webrev after you sync with 
JDK-8173608.  I believe you can revert test/sun/management/jdp and 
test/sun/management/jmxremote tests change and ConnectorBootstrap.java as you 
noted.  Also you can run jdeps —-check on java.rmi, java.management, and 
jdk.management.agent to update its requires and qualified exports.  
java.management should no longer require java.rmi and the qualified exports 
from java.rmi is not needed.

java.management.rmi module-info.java
  32  * @see javax.management.remote.rmi

This @see is not needed.  This package is listed in the exported packages table.

JMXConnectorFactory.java
   I like the ProviderFinder approach to look up the custom providers and 
platform providers; and shared code used by both JMXConnectorFactory and 
JMXConnectorServerFactory which is good.

   Nit: line 481-491 - this is javadoc and probably /* .. */ comment block is 
more appropriate here.

> 
> Some further cleanup of java.base and java.rmi module-info.java
> should become possible once JDK-8173608 is in (as well as moving
> RMIExporter to java.management.rmi - which is not possible yet).
> 

Yes.

> Further cleanup of @modules in tests might become possible as
> well.
> 

Hope there will be a bulk @modules cleanup some time.

Thanks
Mandy

Reply via email to