Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11658 )
Change subject: [sentry] move ParseTableName to table_util ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util-test.cc File src/kudu/util/table_util-test.cc: http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util-test.cc@50 PS1, Line 50: > not your fault, but looks like these two test cases got duplicated between Done http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util-test.cc@54 PS1, Line 54: > "no_table."? Done http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util.h File src/kudu/util/table_util.h: PS1: > nit: How would you feel about putting this in /common instead of /util? Loo Done http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util.h@19 PS1, Line 19: > Should be able to forward declare this If I remove it IWYU will throw an error. http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util.h@29 PS1, Line 29: > What's special about Hive database and Hive table names? Also, what is the Done http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util.cc File src/kudu/util/table_util.cc: http://gerrit.cloudera.org:8080/#/c/11658/1/src/kudu/util/table_util.cc@36 PS1, Line 36: > Given this is no longer 'scoped' under the hms module, I think it makes sen Done -- To view, visit http://gerrit.cloudera.org:8080/11658 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7fff5aaff623ac65a949f6cabebd9eca997f5cdc Gerrit-Change-Number: 11658 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 15 Oct 2018 04:41:52 +0000 Gerrit-HasComments: Yes
