> On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 234 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line234> > > > > CHECK(offers.empty())?
I like CHECK_EQ because it tells us the current size. > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 615 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line615> > > > > 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. Removed the check. > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1248 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line1248> > > > > Why the leading space? fixed > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.hpp, line 316 > > <https://reviews.apache.org/r/14135/diff/1/?file=352201#file352201line316> > > > > CHECK(!tasks.contains(key)) or CHECK_EQ(0, tasks.count(key))? > > > > Would this benefit from including the framework id? done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.hpp, line 328 > > <https://reviews.apache.org/r/14135/diff/1/?file=352201#file352201line328> > > > > CHECK(tasks.contains(key))? Include framework id in the message? done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.hpp, line 460 > > <https://reviews.apache.org/r/14135/diff/1/?file=352201#file352201line460> > > > > framework id? done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.hpp, line 469 > > <https://reviews.apache.org/r/14135/diff/1/?file=352201#file352201line469> > > > > framework id? done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 817 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line817> > > > > CHECK(frameworks.contains(frameworkInfo.id())) done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1221 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line1221> > > > > wrap the comment at 70 lines? done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1320 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line1320> > > > > Why the leading space? copy paste error. fixed. > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/slave/gc.cpp, line 84 > > <https://reviews.apache.org/r/14135/diff/1/?file=352203#file352203line84> > > > > "for gc"? from gc > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1973 > > <https://reviews.apache.org/r/14135/diff/1/?file=352202#file352202line1973> > > > > Use contains? done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/slave/paths.hpp, line 336 > > <https://reviews.apache.org/r/14135/diff/1/?file=352204#file352204line336> > > > > Can we remove this altogether in favor of logging in the caller? killed it. > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/slave/paths.hpp, line 369 > > <https://reviews.apache.org/r/14135/diff/1/?file=352204#file352204line369> > > > > Can we remove this altogether in favor of logging in the caller? killed it. > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 524 > > <https://reviews.apache.org/r/14135/diff/1/?file=352205#file352205line524> > > > > Ditto on CHECK vs ignore and log, but up to you as to whether you'd > > like to change it. done > On Sept. 16, 2013, 4:04 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1998 > > <https://reviews.apache.org/r/14135/diff/1/?file=352205#file352205line1998> > > > > CHECK_NE? done - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14135/#review26139 ----------------------------------------------------------- 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 > >
