Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11422 )
Change subject: [location_awareness] Add location info in ksck report ...................................................................... Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/11422/7/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/11422/7/src/kudu/client/client.h@604 PS7, Line 604: const std::string& location() const; Also, I think we should be very reluctant in adding new calls to the externally faced and published API if we don't have any use case except for our tools using it. Until we have a clear understanding how this is going to be used by an external application, I would prefer not exposing this method. Consider making this method private and adding RemoteKsckCluster into the list of its friends instead. -- To view, visit http://gerrit.cloudera.org:8080/11422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ideff2dd4975c99a1135002624debbbb2620fb95c Gerrit-Change-Number: 11422 Gerrit-PatchSet: 7 Gerrit-Owner: Fengling Wang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Fengling Wang <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 02 Oct 2018 06:54:12 +0000 Gerrit-HasComments: Yes
