Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11248 )
Change subject: KUDU-2469 pt 1: add an IOContext ...................................................................... Patch Set 4: (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) + init_once_.memory_footprint_excluding_this(); Mind restoring this to the old format? I found it to be more readable. 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 always parsed as operator>> hence the separation, but that's not the case anymore. 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: const OrderMode order, fs::IOContext io_context); > I'm not sure I understand this, it's `const` in both the declaration and th I'm trying to find a good reference for this but struggling. What tidy is trying to say is: the _caller_ doesn't care that there's a const on 'order', because order was already passed as a copy anyway. Meaning, unlike for args passed by ref or by pointer, the const here has no effect with respect to the caller. Now, you're right that the const has an effect on the _callee_; it's a self-imposed restriction not to modify 'order'. I think that's why tidy recommends you drop 'const' from the declaration but not the definition: the contract for the caller (the decl) is cleaner without the 'const', but still preserves the const in the contract for the callee (the defn). -- 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: 4 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: Mon, 27 Aug 2018 22:31:40 +0000 Gerrit-HasComments: Yes
