Maxim Smyatkin has posted comments on this change. Change subject: KUDU-78. Fix pb_util functions which return bool to return Status ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4800/1//COMMIT_MSG Commit Message: Line 9: As suggested in ticket, I've refactored four pb_util functions to return Status instead of bool. > Nit: IIRC typically we keep commit message line wrapped to 80 chars or belo Ok, didn't know it. Will have to change default git editor from nano to something showing column positions :) http://gerrit.cloudera.org:8080/#/c/4800/1/src/kudu/util/pb_util.cc File src/kudu/util/pb_util.cc: Line 431: return Status::OK(); > Few of these routines do not seem to be returning no other value than Statu Well, it would be easier for me to do so. But I thought they were designed to return bools at the first place because they were expected to fail in future. And for such cases the error handling code was already added. Maybe I've got it wrong and they can be safely changed to void? Line 443: return Status::Corruption("Error parsing msg", InitializationErrorMessage("parse", *msg)); > This is the only case I don't understand; how could msg->ParseFromZeroCopyS I got the idea that msg may be malformed or something from TODO in pb_util.h: "TODO: change this to return Status - differentiate IO error from bad PB" Perhaps, that's wrong. Let me check it. -- To view, visit http://gerrit.cloudera.org:8080/4800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib760793f6e6da3e357573e525f47b32c79472468 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Maxim Smyatkin <smyatkinma...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Maxim Smyatkin <smyatkinma...@gmail.com> Gerrit-HasComments: Yes
