Alexey Serbin 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 4: Code-Review+1 (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() { return tablet_replica_; } nit: add 'const' specifier? 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: tablet_replica_->Shutdown(); Could tablet_replica_ be nullptr here (e.g., SetUp() bailed out before reaching SetUpReplica())? If so, maybe move call to Shutdown() under if (tablet_replica_) scope? http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.cc@85 PS4, Line 85: apply_pool_->Shutdown(); What about raft_pool_? If it doesn't need to be shutdown here, maybe it's worth adding a comment to explain the reason? http://gerrit.cloudera.org:8080/#/c/16027/4/src/kudu/tablet/tablet_replica-test-base.cc@89 PS4, Line 89: void nit: maybe, make this method return Status? -- 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: 4 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 05:01:36 +0000 Gerrit-HasComments: Yes
