Dan Burkert has posted comments on this change.

Change subject: util: add file cache
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5146/10/src/kudu/util/file_cache-test.cc
File src/kudu/util/file_cache-test.cc:

Line 48: class FileCacheTest : public KuduTest {
> That's already tested indirectly in AssertFdsAndDescriptors(). Since NumDes
Sounds good.


http://gerrit.cloudera.org:8080/#/c/5146/10/src/kudu/util/file_cache.h
File src/kudu/util/file_cache.h:

Line 103:   FileCache(const std::string& cache_name,
> The constructor wasn't actually moving cache_name, so passing by cref elide
but you are using it now, it it could be moved, right?


-- 
To view, visit http://gerrit.cloudera.org:8080/5146
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I26d02f71b0a9644de0b669875941adae5f426345
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to