Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10579 )
Change subject: [tools] Add version info and check to ksck ...................................................................... Patch Set 1: (5 comments) Overall looks good, just a couple of nits. http://gerrit.cloudera.org:8080/#/c/10579/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10579/1//COMMIT_MSG@20 PS1, Line 20: Server nit: Servers ? http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h File src/kudu/tools/ksck.h: http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h@246 PS1, Line 246: boost::optional<std::string> why not a const reference? http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck.h@371 PS1, Line 371: boost::optional<std::string> maybe, return a const reference? http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_remote.cc@199 PS1, Line 199: version_ = resp.status().version_info().version_string(); Maybe, it makes sense to populate version_ field for a server with WRONG_SERVER_UUID health status? http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_results.cc File src/kudu/tools/ksck_results.cc: http://gerrit.cloudera.org:8080/#/c/10579/1/src/kudu/tools/ksck_results.cc@407 PS1, Line 407: Server nit: Servers or Server(s) ? -- To view, visit http://gerrit.cloudera.org:8080/10579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a2bc7138f074ab8d74f21ace2eb2b8018c53f50 Gerrit-Change-Number: 10579 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Sat, 02 Jun 2018 00:00:42 +0000 Gerrit-HasComments: Yes
