Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/12281 )
Change subject: [tools] Describe more table attributes ...................................................................... Patch Set 3: (10 comments) http://gerrit.cloudera.org:8080/#/c/12281/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12281/2//COMMIT_MSG@9 PS2, Line 9: udi > Remove. Done http://gerrit.cloudera.org:8080/#/c/12281/2//COMMIT_MSG@9 PS2, Line 9: `kudu table des > `kudu table describe` Done http://gerrit.cloudera.org:8080/#/c/12281/2//COMMIT_MSG@9 PS2, Line 9: add > adds Done http://gerrit.cloudera.org:8080/#/c/12281/2//COMMIT_MSG@11 PS2, Line 11: fo > for Done http://gerrit.cloudera.org:8080/#/c/12281/2//COMMIT_MSG@13 PS2, Line 13: hen u > text Done http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/common/schema.h@233 PS2, Line 233: type, compression type, and > Can you replace this bool parameter with an enum? See https://abseil.io/tip Done http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/common/schema.cc File src/kudu/common/schema.cc: http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/common/schema.cc@140 PS2, Line 140: ()) > This is sort of silly, but what if the default is "-"? Also, what if the de I update this to what web-ui deal with it: https://github.com/apache/kudu/blob/master/src/kudu/server/webui_util.cc#L50, the non-default value will be -, and any string type value will be wraped by "", like "-". http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/tools/kudu-admin-test.cc@1761 PS2, Line 1761: ->Precision(30) > Can you up the precision here to make sure we test int128 values? I think 3 Done http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/tools/kudu-admin-test.cc@1796 PS2, Line 1796: kAnotherTableId, : }, &stdout, &stderr); > Can you run the tool twice: with and without this flag, to check that the o Done http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/util/int128_util.h File src/kudu/util/int128_util.h: http://gerrit.cloudera.org:8080/#/c/12281/2/src/kudu/util/int128_util.h@18 PS2, Line 18: #pragma once : > Prefer #pragma once. Done -- To view, visit http://gerrit.cloudera.org:8080/12281 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4f8cf682f3a2a30f98edbc4aff6951225d8d81e8 Gerrit-Change-Number: 12281 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 29 Jan 2019 04:57:46 +0000 Gerrit-HasComments: Yes