Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11115 )
Change subject: [location_awareness] Assign locations to registering tablet servers ...................................................................... Patch Set 4: (5 comments) LGTM, just a few nits http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/master_path_handlers.cc File src/kudu/master/master_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/master_path_handlers.cc@117 PS4, Line 117: shared_ptr<TSDescriptor> nit: go a bit further and use auto here? Feel free to ignore, though. http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc File src/kudu/master/ts_descriptor-test.cc: http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc@86 PS4, Line 86: const string location = "/foo-bar0/BAAZ.9-quux"; nit: maybe, add underscore as well http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc@86 PS4, Line 86: const string location = "/foo-bar0/BAAZ.9-quux"; nit: maybe, add underscore as well http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc@103 PS4, Line 103: "/foo$", // Contains the illegal character '$'. nit: maybe, add "\" \"" just in case http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor.cc@113 PS4, Line 113: StripTrailingWhitespace nit: maybe, use StripWhiteSpace() to trim both leading the trailing whitespaces? -- To view, visit http://gerrit.cloudera.org:8080/11115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5eb98823ab7b3b8141b8630196c29c1ebf8e6878 Gerrit-Change-Number: 11115 Gerrit-PatchSet: 4 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Wed, 08 Aug 2018 05:37:08 +0000 Gerrit-HasComments: Yes
