Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11248 )
Change subject: KUDU-2469 pt 1: add an IOContext ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/11248/4/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: http://gerrit.cloudera.org:8080/#/c/11248/4/src/kudu/cfile/cfile_reader.cc@584 PS4, Line 584: size_t size = kudu_malloc_usable_size(this); > Mind restoring this to the old format? I found it to be more readable. Done http://gerrit.cloudera.org:8080/#/c/11248/4/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/11248/4/src/kudu/tablet/delta_tracker.h@307 PS4, Line 307: std::vector<std::shared_ptr<DeltaStore >>* target_stores, > Nit: while you're here, you can change '> >' to '>>'. Pre-c++11, >> was alw Done http://gerrit.cloudera.org:8080/#/c/11248/2/src/kudu/tablet/tablet.h File src/kudu/tablet/tablet.h: http://gerrit.cloudera.org:8080/#/c/11248/2/src/kudu/tablet/tablet.h@774 PS2, Line 774: OrderMode order, fs::IOContext io_context); > I'm trying to find a good reference for this but struggling. Ahh right, if we're passing by copy it doesn't matter to the caller whether or not it's `const` or not. Since this is an enum anyway, I'll get rid of the const both here and at the defn. Thanks for clarifying. -- To view, visit http://gerrit.cloudera.org:8080/11248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I645e10a2fda1f2564326b7052e6a0debc5ad738c Gerrit-Change-Number: 11248 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 28 Aug 2018 06:54:24 +0000 Gerrit-HasComments: Yes
