Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11643 )

Change subject: [test] use LocationInfo in ts_itest_base
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11643/1/src/kudu/integration-tests/ts_itest-base.h
File src/kudu/integration-tests/ts_itest-base.h:

http://gerrit.cloudera.org:8080/#/c/11643/1/src/kudu/integration-tests/ts_itest-base.h@56
PS1, Line 56: std::vector<std::string> non_default_ts_flags
Why not a default value of {} for this? I see that in practice we always 
specify `non_default_ts_flags`, but logically it's consistent to have the {} 
default if `non_default_master_flags` does.


http://gerrit.cloudera.org:8080/#/c/11643/1/src/kudu/integration-tests/ts_itest-base.cc
File src/kudu/integration-tests/ts_itest-base.cc:

http://gerrit.cloudera.org:8080/#/c/11643/1/src/kudu/integration-tests/ts_itest-base.cc@a111
PS1, Line 111:
I guess it was never used?


http://gerrit.cloudera.org:8080/#/c/11643/1/src/kudu/integration-tests/ts_itest-base.cc@134
PS1, Line 134: auto&&
Extra &?


http://gerrit.cloudera.org:8080/#/c/11643/1/src/kudu/integration-tests/ts_itest-base.cc@138
PS1, Line 138: auto&&
Here too.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If6fb1f583e43b9e64e3c396b7be1977546e71347
Gerrit-Change-Number: 11643
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Thu, 11 Oct 2018 16:48:36 +0000
Gerrit-HasComments: Yes

Reply via email to