Todd Lipcon has posted comments on this change. Change subject: KUDU-1932. Run at least one tablet-level test against all block managers. ......................................................................
Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/7252/5//COMMIT_MSG Commit Message: Line 10: Decided to do it against ts_recovery-itest instead. Converted all the nit: this "decided ... instead" only makes sense with recent memory of the discussion around this patch. Looking back on this commit message in the future this won't make any sense, so I think it should just be removed (and maybe just added as a comment in gerrit if you think reviewers will be interested in the current-time context) Line 13: Kudu969Test. some style notes: The contents of the commit message here are somewhat redundant with the content of the patch. Since the changes are relatively straight-forward all things considering, I think it's better to focus on the "why" here -- i.e what additional test coverage are we hoping to get from this change? (not that it needs an essay, since it also links to a JIRA with some motivation). http://gerrit.cloudera.org:8080/#/c/7252/5/src/kudu/integration-tests/ts_recovery-itest.cc File src/kudu/integration-tests/ts_recovery-itest.cc: PS5, Line 76: vector<string> extra_tserver_flags = should be suffixed with '_' Line 79: TsRecoveryITest(); style nit: since this is just a one-line constructor, inline it style nit #2: we usually list public above protected PS5, Line 92: block_mgr_flags nit: should be kBlockManagerFlags per google style guide Line 111: // vector<string> extra_tserver_flags = {}; what's this? -- To view, visit http://gerrit.cloudera.org:8080/7252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Edward Fancher <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Edward Fancher <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
