Adar Dembo has posted comments on this change. Change subject: [timestamp] Add a new TimestampValue class to support the TIMESTAMP_NANOS type ......................................................................
Patch Set 6: (2 comments) Just looked at the API parts. http://gerrit.cloudera.org:8080/#/c/5965/6/src/kudu/common/timestamp_value.h File src/kudu/common/timestamp_value.h: PS6, Line 104: Impala Should probably say "Apache Impala" so it's more clear. PS6, Line 271: // Even though the following methods are public they they are not meant to be used outside : // of Kudu internals. Signatures/names may change at any time. Why aren't they private with friend declarations, then? Would there be too much friendship? I don't see anything here that would be _harmful_ if it were in the public API. I'm a little scared about the inline Compare(), but that's not so bad given that the class is non-PIMPled to begin with. So why exactly do you want to hide them? -- To view, visit http://gerrit.cloudera.org:8080/5965 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5a0763d3a28501b7f9fb0b94552e3227bd5b38cc Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
