Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13649 )
Change subject: [tools] Add get/set extra-configs for CLI tools ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13649/3/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/13649/3/src/kudu/tools/tool_action_table.cc@455 PS3, Line 455: } : } else { : for (const auto& config_name : config_names) { : const string* config_value = FindOrNull(table->extra_configs(), config_name); : if (config_value) { : > Should we instead iterate over the extra configs and to an > unordered_set lookup? I think as written, if --config_names > contains duplicates, we'll print duplicates. > > Might be worth a test too. Thank you for pointing this out. I think it's better to use set here than unordered_set. This is because we expect the result to be alphabetical. -- To view, visit http://gerrit.cloudera.org:8080/13649 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90c790bbfe41a59f621157ff6b3f11d2b8f916e7 Gerrit-Change-Number: 13649 Gerrit-PatchSet: 4 Gerrit-Owner: Yao Xu <oclarms....@gmail.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu <oclarms....@gmail.com> Gerrit-Comment-Date: Thu, 20 Jun 2019 03:47:29 +0000 Gerrit-HasComments: Yes
