Mike Percy has posted comments on this change.

Change subject: mini cluster: Test infrastructure improvements
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6959/3/src/kudu/integration-tests/mini_cluster-itest-base.cc
File src/kudu/integration-tests/mini_cluster-itest-base.cc:

PS3, Line 20: #include <string>
            : #include <vector>
> nit: are these necessary?
Ah, nope. Removed.


http://gerrit.cloudera.org:8080/#/c/6959/3/src/kudu/integration-tests/mini_cluster-itest-base.h
File src/kudu/integration-tests/mini_cluster-itest-base.h:

PS3, Line 38: virtual
> nit: it's better to drop this since there is 'override'
Done


http://gerrit.cloudera.org:8080/#/c/6959/3/src/kudu/tserver/mini_tablet_server.cc
File src/kudu/tserver/mini_tablet_server.cc:

Line 20: #include <utility>
> nit: please also add
it doesn't need the includes because they are pulled in by the corresponding 
header file. But I will add the using expressions


http://gerrit.cloudera.org:8080/#/c/6959/3/src/kudu/tserver/mini_tablet_server.h
File src/kudu/tserver/mini_tablet_server.h:

PS3, Line 26: #include <string>
            : #include <vector>
> nit: could you move these up to precede the list of kudu-specific headers?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bc6a04b113e59243fb788fec15b9935c3dcf069
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to