Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17141 )
Change subject: KUDU-3226 Validate List of Masters for kudu ksck ...................................................................... Patch Set 1: (3 comments) Thanks for the patch Abhishek! Looks like you'll also need to update AdminCliTest.TestAuthzResetCacheIncorrectMasterAddressList in kudu-admin-test.cc to account for the new error message. Also, if you scroll down to the bottom of http://jenkins.kudu.apache.org/job/kudu-gerrit/23429/BUILD_TYPE=LINT/console, there are a few updates the Kudu linter would like you to make http://gerrit.cloudera.org:8080/#/c/17141/1/src/kudu/tools/tool_action_common.cc File src/kudu/tools/tool_action_common.cc: http://gerrit.cloudera.org:8080/#/c/17141/1/src/kudu/tools/tool_action_common.cc@652 PS1, Line 652: std:: nit: because of the 'using' declarations around L200, this can just be unordered_set<string> and string below http://gerrit.cloudera.org:8080/#/c/17141/1/src/kudu/tools/tool_action_common.cc@659 PS1, Line 659: unique_masters.find(master_addresses->at(i)) != unique_masters.end() nit: you can use the ContainsKey() function from gutil/map-util.h http://gerrit.cloudera.org:8080/#/c/17141/1/src/kudu/tools/tool_action_common.cc@656 PS1, Line 656: for (int i = 0; i<master_addresses->size(); i++){ : if ((master_addresses->at(i)).empty()) : continue; : if (unique_masters.find(master_addresses->at(i)) != unique_masters.end()){ : duplicate_masters = duplicate_masters.append(master_addresses->at(i))+" "; : } : else : unique_masters.insert(master_addresses->at(i)); : } > It's best to not modify out parameter master_addresses unless we are certai +1 nit: as well, we generally follow the Google Style Guide, which suggests consistent usage of curly braces for if/else statements: https://google.github.io/styleguide/cppguide.html#Conditionals Same elsewhere. -- To view, visit http://gerrit.cloudera.org:8080/17141 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f3b2b7dcf2ac78cb95cf43242651e3ce8fddf6f Gerrit-Change-Number: 17141 Gerrit-PatchSet: 1 Gerrit-Owner: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 02 Mar 2021 02:15:18 +0000 Gerrit-HasComments: Yes
