Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10151 )
Change subject: [WIP] [tools] ksck improvements [6/n]: Refactor printing ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc File src/kudu/tools/ksck_results.cc: http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@88 PS5, Line 88: const string labels = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; static? It seems this function can be entered multiple times during kudu CLI lifecycle. Also, maybe change this to 'static const char* const' and use (sizeof(xxx)-1) instead of size(). http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@124 PS5, Line 124: string ServerHealthToString(KsckServerHealth sh) { nit: maybe, return 'const char* const' instead? http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@177 PS5, Line 177: // Then, summarize the tablets by table nit: add a stop http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@268 PS5, Line 268: nit: two extra spaces http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@269 PS5, Line 269: "Name", "Status", "Total Tablets", : "Healthy", "Recovering", "Under-replicated", "Unavailable" consider adding replication factor, as it's done in https://gerrit.cloudera.org/#/c/10054/ http://gerrit.cloudera.org:8080/#/c/10151/5/src/kudu/tools/ksck_results.cc@273 PS5, Line 273: switch (ts.TableStatus()) { : case KsckCheckResult::HEALTHY: : status = "HEALTHY"; : break; : case KsckCheckResult::RECOVERING: : status = "RECOVERING"; : break; : case KsckCheckResult::UNDER_REPLICATED: : status = "UNDER-REPLICATED"; : break; : default: : status = "UNAVAILABLE"; : break; : } maybe, separate this into a function? -- To view, visit http://gerrit.cloudera.org:8080/10151 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8de619996b6cd753e6a9c01b1b60810a873e609 Gerrit-Change-Number: 10151 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Tue, 24 Apr 2018 23:52:34 +0000 Gerrit-HasComments: Yes
