Andrew Wong has posted comments on this change.

Change subject: external minicluster: expand EMC dir usage
......................................................................


Patch Set 16:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6845/15/src/kudu/integration-tests/delete_table-itest.cc
File src/kudu/integration-tests/delete_table-itest.cc:

Line 1063:   ASSERT_OK(cluster_->master()->DeleteFromDisk());
> Would it make sense to just provide a DeleteFromDisk() method on the daemon
Done


http://gerrit.cloudera.org:8080/#/c/6845/15/src/kudu/integration-tests/external_mini_cluster.cc
File src/kudu/integration-tests/external_mini_cluster.cc:

Line 253:   string data_path = "data";
> I think we should add a check s.t. if dir_index is not passed then we must 
Done


PS15, Line 254: if (dir_index) {
> not a big deal but you can just say if (dir_index) here if you want
Done


PS15, Line 255: CHECK
> CHECK_LE
Done


PS15, Line 255: LE(*dir_index, 
> it's up to you but fyi you can also use the syntax *dir_index
Done


http://gerrit.cloudera.org:8080/#/c/6845/15/src/kudu/integration-tests/external_mini_cluster.h
File src/kudu/integration-tests/external_mini_cluster.h:

Line 20: 
> nit: boost/optional isn't a standard C header, it should either go with the
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to