Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/9787 )
Change subject: KUDU-2364 Add extra check in ksck for tserver ID ...................................................................... Patch Set 11: (8 comments) http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck.h File src/kudu/tools/ksck.h: http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck.h@529 PS10, Line 529: : static Status PrintTabletServerSummaries( : const std::vector<TabletServerSummary>& tablet_server_summaries, : std::ostream& out); > Nit: when a declaration gets too long like this, rather than split the type Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck.cc File src/kudu/tools/ksck.cc: http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck.cc@42 PS10, Line 42: #include "kudu/gutil/strings/util.h" > Nit: alphabetical sorting would dictate that this be above gutil/strings/.. Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck.cc@258 PS10, Line 258: auto& ts : tablet_s > Nit: could use 'auto' here instead. More terse. Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote-test.cc File src/kudu/tools/ksck_remote-test.cc: http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote-test.cc@242 PS10, Line 242: string new_uuid = new_tablet_server.uuid(); > Nit: don't need std:: prefixing here or below. Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote-test.cc@245 PS10, Line 245: > Don't need this anymore. Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote.cc@107 PS10, Line 107: RETURN_NOT_OK_PREPEND(generic_proxy_->GetStatus(req, &resp, &rpc), : "could not get status from server"); > Combine these? No need for 's' anymore. Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote.cc@109 PS10, Line 109: string response_uuid = resp.status().node_instance().permanent_uuid(); > Nit: no std:: prefix. Done http://gerrit.cloudera.org:8080/#/c/9787/10/src/kudu/tools/ksck_remote.cc@123 PS10, Line 123: RETURN_NOT_OK_PREPEND(ts_proxy_->ListTablets(req, &resp, &rpc), > Don't need this line anymore. Done -- To view, visit http://gerrit.cloudera.org:8080/9787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia2c18ba7af8eaa6f5e4d7842f18754d2c1e32526 Gerrit-Change-Number: 9787 Gerrit-PatchSet: 11 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Wed, 28 Mar 2018 17:39:05 +0000 Gerrit-HasComments: Yes
