Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9137 )
Change subject: internal_mini_cluster: support Cluster/LogVerifier ...................................................................... Patch Set 11: Code-Review+1 (5 comments) Looks good! Thanks for doing this. I just have a few nits. http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/consensus/log_reader.cc File src/kudu/consensus/log_reader.cc: http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/consensus/log_reader.cc@86 PS11, Line 86: RETURN_NOT_OK how about: RETURN_NOT_OK_PREPEND(log_reader->Init(tablet_wal_dir), "Unable to initialize log reader"); http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/consensus/log_reader.cc@95 PS11, Line 95: metric_entity, nit: unusual line wrapping http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/integration-tests/log_verifier.h File src/kudu/integration-tests/log_verifier.h: http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/integration-tests/log_verifier.h@74 PS11, Line 74: const cluster::MiniCluster* nit: const cluster::MiniCluster* const ? http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/mini-cluster/external_mini_cluster.h File src/kudu/mini-cluster/external_mini_cluster.h: http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/mini-cluster/external_mini_cluster.h@267 PS11, Line 267: UUIDforTS nit: how about UuidForTS() or GetUuidForTabletServer() ... the lowercase "for" is a bit unusual http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/tablet/tablet_bootstrap.cc@708 PS11, Line 708: index nit: mind naming this log_index? Kudu has all kinds of indexes everywhere, including loop indexes -- To view, visit http://gerrit.cloudera.org:8080/9137 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228a6e3ba1a42db4e243ffdc5116f0c60ee04a84 Gerrit-Change-Number: 9137 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 13 Feb 2018 01:28:13 +0000 Gerrit-HasComments: Yes
