Grant Henke has posted comments on this change. Change subject: env: add WriteV() API ......................................................................
Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: Line 478: } > Any particular reason to preserve this? It's a private function; if it's no Done Line 480: Status CFileWriter::WriteRawDataV(const vector<Slice> &data) { > vector<Slice>& Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.h File src/kudu/cfile/cfile_writer.h: Line 189: Status WriteRawDataV(const vector<Slice> &data); > Should be vector<Slice>& (& next to the type). Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 1063: virtual Status AppendV(const std::vector<Slice>& data) OVERRIDE; > Don't need std:: prefix here. Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: Line 163: // Force short reads to half the slice length > You mean writes? Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env.h File src/kudu/util/env.h: Line 448: virtual Status AppendV(const std::vector<Slice> &data) = 0; > std::vector<Slice>& Done Line 543: virtual Status WriteV(uint64_t offset, const vector<Slice> &data) = 0; > Need std:: prefix here. Also vector<Slice>& Done 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. Done Line 571: Status s = DoWriteV(fd_, filename_, filesize_, data); > Hmm, this seems weird; shouldn't we RETURN_NOT_OK() and not touch filesize_ Done -- 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: Grant Henke <granthe...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
