[GitHub] incubator-rocketmq pull request #152: [ROCKETMQ-278] Add clusterlist cmd by ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/152#discussion_r139871853
  
--- Diff: 
namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
 ---
@@ -63,11 +65,36 @@ public RouteInfoManager() {
 this.filterServerTable = new HashMap(256);
 }
 
-public byte[] getAllClusterInfo() {
-ClusterInfo clusterInfoSerializeWrapper = new ClusterInfo();
-
clusterInfoSerializeWrapper.setBrokerAddrTable(this.brokerAddrTable);
-
clusterInfoSerializeWrapper.setClusterAddrTable(this.clusterAddrTable);
-return clusterInfoSerializeWrapper.encode();
+public byte[] getAllClusterInfo(String cluster) {
--- End diff --

I agree @shroman here


---

[GitHub] incubator-rocketmq pull request #152: [ROCKETMQ-278] Add clusterlist cmd by ...

[shroman]

Github user shroman commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/152#discussion_r139078626
  
--- Diff: 
namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
 ---
@@ -63,11 +65,36 @@ public RouteInfoManager() {
 this.filterServerTable = new HashMap(256);
 }
 
-public byte[] getAllClusterInfo() {
-ClusterInfo clusterInfoSerializeWrapper = new ClusterInfo();
-
clusterInfoSerializeWrapper.setBrokerAddrTable(this.brokerAddrTable);
-
clusterInfoSerializeWrapper.setClusterAddrTable(this.clusterAddrTable);
-return clusterInfoSerializeWrapper.encode();
+public byte[] getAllClusterInfo(String cluster) {
--- End diff --

You change the signature of the public class here.
How about having another method -- `public byte[] getAllClusterInfo()`? And 
you won't need calling `getAllClusterInfo(null)` then too ;)


---