pkuwm commented on a change in pull request #1199:
URL: https://github.com/apache/helix/pull/1199#discussion_r463897625
##########
File path: helix-core/src/main/java/org/apache/helix/InstanceType.java
##########
@@ -22,7 +22,7 @@
import java.util.Arrays;
import java.util.List;
-import org.apache.helix.monitoring.mbeans.MonitorDomainNames;
+import org.apache.helix.monitoring.common.mbeans.MonitorDomainNames;
Review comment:
Thanks for pointing it out. I think this does remove classes from helix
jars: helix-core.jar. Files are not removed from code base, but from the
exported jars. Anyway, changed removed to resolved.
The classes in metrics-common have the same package path
org.apache.helix.monitoring as those classes in helix-core. The challenge is,
with existing bundle plugin/strategy in helix, there is no way to exclude the
metric-common's classes in helix-core.jar.
Even we could change the bundle plugin to support class filtering, I still
don't think it is clean if we still put same package classes in different jars.
The right way for different modules/jars should have different package
paths, eg.:
metrics-common: org.apache.helix.monitoring.common
helix-core: org.apache.helix.monitoring
With these different package paths, we could cleanly avoid exporting classes
of metrics-common to helix-core.jar.
This is the reason why the PR would like to differentiate the package paths
for different modules/jars.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
