[GitHub] [helix] pkuwm commented on a change in pull request #1199: Resolve duplicate classes in different helix jars

[GitBox <gi...@apache.org>]

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 out the misleading title. Changed it 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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org