Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17267 )
Change subject: [master][tool] KUDU-2181 Tool to orchestrate adding a master ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_master.cc File src/kudu/tools/tool_action_master.cc: http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_master.cc@349 PS2, Line 349: // Get the flags that'll be needed to bring up new master and for system catalog copy. : GFlagsMap flags_map = GetFlagsMap(); > Btw we are passing through all the flags here to bring up the master > specified by the user and only omitting the ones that are specific for this > command (wait_secs). So the ones like block_manager will be passed through > as well if the user specifies them and not the only ones listed in the help. What does "passed through" mean here? Is that to say that the flag will have all Master's flags passed to it? If so, could you mention that in the command description? > Space is not suitable too since a gflag could be passed as --flag val as well Not sure I follow this. If we tell users to supply the arguments as --flag=val, why can't we delimit on space? One of the downsides with b is that it makes reasoning about the other flags a bit confusing as well, not that they'd actually be different from the Master's flags, but it sits better with me to have the Master flags be a bit more explicit. http://gerrit.cloudera.org:8080/#/c/17267/2/src/kudu/tools/tool_action_master.cc@363 PS2, Line 363: [[maybe_unused]] > underscore _ Is this required for a lint check or something? I was under the impression that we cannot use _ even if we wanted to -- it's not assigned a name and is unusable. If so, why do we need to explicitly tag it as unused, given it's an unusable variable? -- To view, visit http://gerrit.cloudera.org:8080/17267 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f99cf2b3f1738c4127c7e7288beab61daf42e7b Gerrit-Change-Number: 17267 Gerrit-PatchSet: 2 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 05 Apr 2021 19:31:08 +0000 Gerrit-HasComments: Yes
