----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14135/#review26139 -----------------------------------------------------------
Ship it! src/master/master.hpp <https://reviews.apache.org/r/14135/#comment51023> CHECK(!tasks.contains(key)) or CHECK_EQ(0, tasks.count(key))? Would this benefit from including the framework id? src/master/master.hpp <https://reviews.apache.org/r/14135/#comment51024> CHECK(tasks.contains(key))? Include framework id in the message? src/master/master.hpp <https://reviews.apache.org/r/14135/#comment51025> framework id? src/master/master.hpp <https://reviews.apache.org/r/14135/#comment51026> framework id? src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51029> CHECK(offers.empty())? src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51031> Mention that it was discarded? In retrospect I think I should not have made this a CHECK. Rather we should ignore it and LOG(WARNING), up to you whether you want to fix that now. src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51034> CHECK(frameworks.contains(frameworkInfo.id())) src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51035> wrap the comment at 70 lines? src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51038> Why the leading space? src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51039> Why the leading space? src/master/master.cpp <https://reviews.apache.org/r/14135/#comment51040> Use contains? src/slave/gc.cpp <https://reviews.apache.org/r/14135/#comment51041> "for gc"? src/slave/paths.hpp <https://reviews.apache.org/r/14135/#comment51043> Can we remove this altogether in favor of logging in the caller? src/slave/paths.hpp <https://reviews.apache.org/r/14135/#comment51044> Can we remove this altogether in favor of logging in the caller? src/slave/slave.cpp <https://reviews.apache.org/r/14135/#comment51045> Ditto on CHECK vs ignore and log, but up to you as to whether you'd like to change it. src/slave/slave.cpp <https://reviews.apache.org/r/14135/#comment51046> CHECK_NE? - Ben Mahler On Sept. 14, 2013, 12:10 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14135/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2013, 12:10 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Jie Yu. > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/master/master.hpp 19be184610fd9d75e47f44dc804afefe0babe7a6 > src/master/master.cpp 30abe9d4f4576a961c3d427f083fc2ce5c92b601 > src/slave/gc.cpp 827534f1cfbc848fce799340dd0ff8dcff3f8a11 > src/slave/paths.hpp 3a02a0d6a977b1af7c7f7b9d75bd4a44b0c53c2b > src/slave/slave.cpp cefb42004da15d390c276ad5337b558ba5bf8e59 > > Diff: https://reviews.apache.org/r/14135/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
