Edward Fancher has posted comments on this change.

Change subject: KUDU-1932. Run at least one tablet-level test against all block 
managers.
......................................................................


Patch Set 5:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/7252/5//COMMIT_MSG
Commit Message:

PS5, Line 8: .
> nit: don't end your commit message with a period and don't wrap the first l
Done


PS5, Line 8: .
> Just for reference, there is useful info on writing commit messages: https:
Thanks!


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 
Done


Line 13: Kudu969Test.
> some style notes:
Replaced with 
Adding current block managers (file and log) to get extra coverage of block 
managers. The file block manager especially has some thin coverage since the 
log manager is the default on linux machines. Additionally, the file block 
manager is required on macs, so it's easy for someone working just on a linux 
machine to break developers who are using a mac to build and test.


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 '_'
Done.


Line 79:   TsRecoveryITest();
> style nit: since this is just a one-line constructor, inline it
Done.


PS5, Line 92: block_mgr_flags
> nit: should be kBlockManagerFlags per google style guide
Done


Line 111:   // vector<string> extra_tserver_flags = {};
> what's this?
Oops. Removed.


Line 195:   
extra_tserver_flags_with_crash.push_back({"--fault_crash_during_log_replay=0.05"});
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


Line 243:   extra_tserver_flags.push_back({"--log_segment_size_mb=1",
> warning: use emplace_back instead of push_back [modernize-use-emplace]
Done


-- 
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: Alexey Serbin <[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

Reply via email to