Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12849 )
Change subject: KUDU-1711: ColumnSchema supports storing column comment ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/12849/5/src/kudu/client/schema.h File src/kudu/client/schema.h: http://gerrit.cloudera.org:8080/#/c/12849/5/src/kudu/client/schema.h@29 PS5, Line 29: #include <boost/optional/optional.hpp> Hmm, actually this isn't going to work, because we expect third party applications to include this header directly, and this would drag in a boost dependency. So I guess we're going to have to use a regular std::string for the KuduColumnSchema constructor, unless you can forward declare boost::optional<std::string> and boost::none. http://gerrit.cloudera.org:8080/#/c/12849/5/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/12849/5/src/kudu/common/schema.h@347 PS5, Line 347: // "no comment" and "empty comment" are the same. Nit: I think it'd be clearer if you removed the this-> references. -- To view, visit http://gerrit.cloudera.org:8080/12849 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82d3228100c262cc4a5c63eaf696927c88bc8990 Gerrit-Change-Number: 12849 Gerrit-PatchSet: 5 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Wed, 27 Mar 2019 17:09:38 +0000 Gerrit-HasComments: Yes
