Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8989 )
Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735 ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2428 PS3, Line 2428: vector<TServerDetails*> tservers; > nit: this vector isn't really needed anymore Done http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2430 PS3, Line 2430: ASSERT_GE(tservers.size(), 3); > nit: I don't think this is really required but if so it can just be ASSERT_ Done http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2432 PS3, Line 2432: tservers[0] > nit: how about: Good idea! http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2477 PS3, Line 2477: attemp > typo: attempt Done http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2497 PS3, Line 2497: ASSERT_EVENTUALLY([&] { > nit: this ASSERT_EVENTUALLY can go outside the loop Done -- To view, visit http://gerrit.cloudera.org:8080/8989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e Gerrit-Change-Number: 8989 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Comment-Date: Fri, 12 Jan 2018 23:21:42 +0000 Gerrit-HasComments: Yes
