Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10656 )
Change subject: KUDU-2459: add placeholder names to some CREATE TABLE statements ...................................................................... Patch Set 14: (2 comments) http://gerrit.cloudera.org:8080/#/c/10656/14/src/kudu/master/master_path_handlers.cc File src/kudu/master/master_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/10656/14/src/kudu/master/master_path_handlers.cc@265 PS14, Line 265: // Not all Kudu tablenames are also valid Impala identifiers. We need to replace such names : // with a placeholder when they are used as Impala identifiers, for example as Impala tablename : // in the Kudu Web UI. Valid Impala identifiers conform to the following rules: These lines look like they're too long: please wrap at 80 characters. http://gerrit.cloudera.org:8080/#/c/10656/14/src/kudu/master/master_path_handlers.cc@273 PS14, Line 273: string impala_name = "<placeholder>"; Style nit: when you want to emphasize that a comment applies to a particular block of code, format it like this: <some other code> <comment> <applicable block of code> <other code> What's important here? That the comment and the block of code aren't separated by an empty line, and that the two are separated from the code before and after with empty lines. This helps emphasize that the comment and the code should be considered together. -- To view, visit http://gerrit.cloudera.org:8080/10656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9e5242318589452a0507d3973b9be3e6a186a69 Gerrit-Change-Number: 10656 Gerrit-PatchSet: 14 Gerrit-Owner: Shriya Gupta <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Shriya Gupta <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Wed, 01 Aug 2018 18:58:08 +0000 Gerrit-HasComments: Yes
