> 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