Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8307 )
Change subject: [webui] Add templates for tserver webui ...................................................................... Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.h File src/kudu/tserver/tserver_path_handlers.h: http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.h@27 PS7, Line 27: template <class T> class scoped_refptr; > warning: invalid case style for class 'scoped_refptr' [readability-identifi :( http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc File src/kudu/tserver/tserver_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@170 PS7, Line 170: transaction_json["desc"] = std::move(inflight_tx.description()); > warning: std::move of the const expression has no effect; remove std::move( Done http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@172 PS7, Line 172: transaction_json["trace"] = std::move(inflight_tx.trace_buffer()); > warning: std::move of the const expression has no effect; remove std::move( Done http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@214 PS7, Line 214: map<TabletStatePB, int> tablet_statuses; > nit: maybe rename this to tablet_states? Done http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@215 PS7, Line 215: for (const scoped_refptr<TabletReplica>& replica : replicas) { > Can this loop be merged with the other loop over `replicas`? Seems like the Done http://gerrit.cloudera.org:8080/#/c/8307/7/src/kudu/tserver/tserver_path_handlers.cc@240 PS7, Line 240: tablet_detail_json["target"] = Substitute("/tablet?id=$0", status.tablet_id()); > Does the /tablet page exist if replica exists but the tablet doesn't exist? Technically the /tablet page does exist if the replica exists but the tablet doesn't, but there's not going to be anything useful about it anymore. -- To view, visit http://gerrit.cloudera.org:8080/8307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I99e08be9aa8abddd51ada61683b6f75190a00b5c Gerrit-Change-Number: 8307 Gerrit-PatchSet: 7 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Mon, 30 Oct 2017 22:10:54 +0000 Gerrit-HasComments: Yes
