Adar Dembo has posted comments on this change. Change subject: KUDU-1474: single to multi-master deployment migration ......................................................................
Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/gutil/strings/join.h File src/kudu/gutil/strings/join.h: Line 204: string JoinStrings(const CONTAINER& components, > some comment doc here would be good. Perhaps a different name as well, like Done http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_action.cc File src/kudu/tools/tool_action.cc: Line 45: msg += "Action can be one of the following:\n"; > this function only seems useful/sensical if 'sub_actions' is non-empty, but Yeah, L63 is dead code; there's no invocation of BuildNonLeafActionHelpString() with an empty chain. Removed it. http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_action.h File src/kudu/tools/tool_action.h: Line 98: Status CheckNoMoreArgs(const std::vector<Action>& chain, CONTAINER args) { > I think this can take the CONTAINER by const ref. Done http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: Line 124: RETURN_NOT_OK(env_util::CopyFile(env, cmeta_filename, backup_filename, opts)); > worth a LOG(INFO) about 'backing up current config to <filename>' Done http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_main.cc File src/kudu/tools/tool_main.cc: Line 69: vector<Action> sub_actions = cur->sub_actions; > is this mutated? or can it be const auto&? Done Line 130: FLAGS_helpfull || > it seems like it would be useful to still allow --helpfull or something to Alright. I'm not sure how useful the real help is, but I won't block it. -- To view, visit http://gerrit.cloudera.org:8080/3880 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89c741381ced3731736228cd07fe85106ae72541 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
