Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/11313 )
Change subject: [location_awareness] Add 'location' column in tserver list ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11313/2/src/kudu/tools/tool_action_tserver.cc File src/kudu/tools/tool_action_tserver.cc: http://gerrit.cloudera.org:8080/#/c/11313/2/src/kudu/tools/tool_action_tserver.cc@141 PS2, Line 141: location > Good catch but could you tell me why would it be cleaner? Thought we might This is just my subjective opinion. Using constants for strings that are used more than once has two advantages. One - compiler will catch typos and spelling errors. Two - if the name of the column changes for some reason (even if this is highly unlikely), we will avoid the risk of forgetting to change the string in any of places where it is used. -- To view, visit http://gerrit.cloudera.org:8080/11313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If6c9dc8bd08b8d907111fac850fc6fd2c2b96fb8 Gerrit-Change-Number: 11313 Gerrit-PatchSet: 4 Gerrit-Owner: Fengling Wang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Fengling Wang <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 28 Aug 2018 22:49:41 +0000 Gerrit-HasComments: Yes
