Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17266 )
Change subject: [mini-cluster] Refactor to expose flags of ExternalMaster ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/17266/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17266/1//COMMIT_MSG@10 PS1, Line 10: are nit: were http://gerrit.cloudera.org:8080/#/c/17266/1/src/kudu/mini-cluster/external_mini_cluster.h File src/kudu/mini-cluster/external_mini_cluster.h: http://gerrit.cloudera.org:8080/#/c/17266/1/src/kudu/mini-cluster/external_mini_cluster.h@729 PS1, Line 729: GetFlags Is this method shading ExternalDaemon::GetFlags()? If so, I guess it might lead to unexpected behavior when working with base pointers/references -- just wanted to make sure that's what we really want here. If shading isn't desired, consider using different names/signatures for ExternalDaemon::GetFlags() and ExternalMaster::GetFlags(). http://gerrit.cloudera.org:8080/#/c/17266/1/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/17266/1/src/kudu/mini-cluster/external_mini_cluster.cc@1012 PS1, Line 1012: info_path Is it possible to derive 'info_path' from path to the first data directory, the same as it was prior to this change? http://gerrit.cloudera.org:8080/#/c/17266/1/src/kudu/mini-cluster/external_mini_cluster.cc@1493 PS1, Line 1493: static vector<string> kFlags; : if (!UseLargeKeys()) { nit: maybe, use ternary conditional to define kFlags instead of this declare/assign sequence? -- To view, visit http://gerrit.cloudera.org:8080/17266 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibeff3b0d6bc0021ce2aa50e8022542fb32250e07 Gerrit-Change-Number: 17266 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 05 Apr 2021 21:49:32 +0000 Gerrit-HasComments: Yes
