Adar Dembo has posted comments on this change. Change subject: WIP: Add a new TIMESTAMP type ......................................................................
Patch Set 3: (1 comment) Just passing through... http://gerrit.cloudera.org:8080/#/c/5819/3/src/kudu/util/timestamp_value.h File src/kudu/util/timestamp_value.h: Since this is client facing, here are a few other things to keep in mind: 1. Consider _not_ using inline functions, which place additional restrictions on the functions/fields they call. 2. Since it's not possible to add new fields once this has shipped, consider adding an extra int32_t or something of unused data as 'reserved', to be used in the future (for bit flags or something like that). For more information on binary compatibility dos and donts, check out: https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B -- To view, visit http://gerrit.cloudera.org:8080/5819 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I63271adf6b87f048e30bf2d6d04c758884752afc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
