Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16027 )
Change subject: tablet_replica-test: refactor to pull out some useful code ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.h File src/kudu/tablet/tablet_replica-test-base.h: http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.h@63 PS4, Line 63: const scoped_refptr<TabletReplica>& tablet_replica() const { return tablet_replica_; } > nit: add 'const' specifier? Done http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.cc File src/kudu/tablet/tablet_replica-test-base.cc: http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.cc@83 PS4, Line 83: ASSERT_OK(SetUpReplica()); > Could tablet_replica_ be nullptr here (e.g., SetUp() bailed out before reac I don't think that's possible. If SetUp() fails, the test exits regardless. http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.cc@85 PS4, Line 85: > What about raft_pool_? If it doesn't need to be shutdown here, maybe it's Done http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.cc@89 PS4, Line 89: ap > nit: maybe, make this method return Status? Done -- To view, visit http://gerrit.cloudera.org:8080/16027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I912221d73b4d83bb967d91ca7592618b4a89d74c Gerrit-Change-Number: 16027 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 03 Jun 2020 19:32:30 +0000 Gerrit-HasComments: Yes
