David Ribeiro Alves has posted comments on this change. Change subject: KUDU-980 - Fix timestamp printing in c++ ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/965/1/src/kudu/common/types.h File src/kudu/common/types.h: Line 331: static const char* kDateMicrosAndTzFormat = "%s.%06d GMT"; > Unless we have a reason otherwise, this should probably be ISO-8601 (should the time format is the same as impala's (no 'T'), which deviates from the standard. We do print the timezone to make it clear that all of our times are in GMT, I can submit a follow up patch to elide it from both client's (like impala does) if we prefer to do so. Line 350: snprintf(time, sizeof(time), kDateMicrosAndTzFormat, time_up_to_secs, remaining_micros); > can we get some test coverage for this? Done -- To view, visit http://gerrit.cloudera.org:8080/965 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97eb088a4d0ef082ab9c7e3cca40e809b0f29934 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
