----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16433/#review31944 -----------------------------------------------------------
src/log/main.cpp <https://reviews.apache.org/r/16433/#comment60701> Use 'count' or a hashmap and 'contains'? src/log/main.cpp <https://reviews.apache.org/r/16433/#comment60700> Kill the 'else', just leave the return at the top level. src/log/main.cpp <https://reviews.apache.org/r/16433/#comment60699> It's not clear that pulling this out of Tool is necessary or useful. How about we just do 'tool->execute(argc, argv)' and have it load any command line flags as necessary? src/log/tool/initialize.cpp <https://reviews.apache.org/r/16433/#comment60702> Consider being even more explicit in your message, for example, say "missing 'path' command line flag" or "missing '--path'". src/log/tool/initialize.cpp <https://reviews.apache.org/r/16433/#comment60703> Think about the potential user of this tool (who likely does not know anything about futures or libprocess): "Failed to get status of replica (discarded future)" And clean up other errors too. src/log/tool/read.cpp <https://reviews.apache.org/r/16433/#comment60704> See comment in initialize.cpp. src/log/tool/read.cpp <https://reviews.apache.org/r/16433/#comment60705> See comments in initialize.cpp. src/tests/log_tests.cpp <https://reviews.apache.org/r/16433/#comment60706> Why is this in the review? - Benjamin Hindman On Dec. 21, 2013, 9:17 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16433/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2013, 9:17 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Jiang Yan Xu. > > > Bugs: MESOS-736 > https://issues.apache.org/jira/browse/MESOS-736 > > > Repository: mesos-git > > > Description > ------- > > See summary. I also pulled storage related code out (I haven't changed them) > from replica.cpp. > > > Diffs > ----- > > src/Makefile.am ddc43bd > src/log/leveldb.hpp PRE-CREATION > src/log/leveldb.cpp PRE-CREATION > src/log/main.cpp f07bd10 > src/log/replica.hpp d1f5ead > src/log/replica.cpp 82c2157 > src/log/storage.hpp PRE-CREATION > src/log/tool.hpp PRE-CREATION > src/log/tool/initialize.hpp PRE-CREATION > src/log/tool/initialize.cpp PRE-CREATION > src/log/tool/read.hpp PRE-CREATION > src/log/tool/read.cpp PRE-CREATION > src/tests/log_tests.cpp ff5f86c > > Diff: https://reviews.apache.org/r/16433/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
