Alexey Serbin has posted comments on this change. Change subject: KUDU-1600 (part 1): bump to CFile version 2 ......................................................................
Patch Set 3: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/5678/1/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: PS1, Line 80: 2_t len = > I don't think it's really problematic -- it'll just feed the empty block to That sounds good to me -- thank you for the clarification. http://gerrit.cloudera.org:8080/#/c/5678/2/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: PS2, Line 71: const char kMagicStringV1[] = "kuducfil"; : const char kMagicStringV2[] = "kuducfl2"; : const int kMagicLength = 8; > meh... I think it's easy enough to look at them and see that they're the sa I was elaborating on the compile-time check that Adar mentioned in PS1 comment. :) If there is decent run-time test coverage, then it's safe, of course. -- To view, visit http://gerrit.cloudera.org:8080/5678 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I13999de3d406fb184cea3e6e8d52ffd80b8d8ee3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
