Adar Dembo has posted comments on this change. Change subject: env: add WriteV() API ......................................................................
Patch Set 1: (10 comments) http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: PS1, Line 476: Status CFileWriter::WriteRawData(const Slice& data) { : return WriteRawDataV({ data }); : } Any particular reason to preserve this? It's a private function; if it's not being used anymore, let's remove it. PS1, Line 480: vector<Slice> & vector<Slice>& http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.h File src/kudu/cfile/cfile_writer.h: PS1, Line 189: vector<Slice> & Should be vector<Slice>& (& next to the type). http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/fs/file_block_manager.cc File src/kudu/fs/file_block_manager.cc: PS1, Line 229: std:: Don't need. http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS1, Line 1063: std:: Don't need std:: prefix here. http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: PS1, Line 163: reads You mean writes? http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env.h File src/kudu/util/env.h: PS1, Line 448: & std::vector<Slice>& PS1, Line 543: vector Need std:: prefix here. Also vector<Slice>& http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 408: // Convert the results into the iovec vector to request Nit: terminate comments that are English sentences with a period. Line 571: Status s = DoWriteV(fd_, filename_, filesize_, data); Hmm, this seems weird; shouldn't we RETURN_NOT_OK() and not touch filesize_ or pending_sync_ on failure? -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
