----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25111/#review51687 -----------------------------------------------------------
src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90216> I think this exceeds 70 char comment limit. src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90223> I don't think you need a bool either. It should either succeed or not, and when it doesn't you throw an error anyways (doesn't contain info.id). So I think it should be a Try<Nothing> src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90218> Period in the end src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90219> Return Error instead. src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90221> space between for ( src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90222> space bewteen if ( src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90220> We have a Unreachable define you can throw. #include <stout/unreachable.hpp> return UNREACHABLE(); src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90224> s/NOTE: // src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90225> period in the end. src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90230> Period in the end. src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90226> space between for loop. src/master/master.hpp <https://reviews.apache.org/r/25111/#comment90227> space between if ( src/master/master.cpp <https://reviews.apache.org/r/25111/#comment90228> I assume you're not finished here. src/slave/slave.cpp <https://reviews.apache.org/r/25111/#comment90231> You already have a Try in the return type, doesn't need another try in the method name. src/slave/slave.cpp <https://reviews.apache.org/r/25111/#comment90232> & refernece operator is next to the > ie: const Option<Attributes>& attrs, src/slave/slave.cpp <https://reviews.apache.org/r/25111/#comment90233> No ending period in error messages. src/slave/slave.cpp <https://reviews.apache.org/r/25111/#comment90234> You're checking if the new resources is a subset though? src/slave/slave.cpp <https://reviews.apache.org/r/25111/#comment90236> Please add periods in all the comments in this method src/slave/slave.cpp <https://reviews.apache.org/r/25111/#comment90235> Space before the last return - Timothy Chen On Aug. 27, 2014, 7:48 p.m., Patrick Reilly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25111/ > ----------------------------------------------------------- > > (Updated Aug. 27, 2014, 7:48 p.m.) > > > Review request for mesos, Adam B and Benjamin Hindman. > > > Bugs: MESOS-1739 > https://issues.apache.org/jira/browse/MESOS-1739 > > > Repository: mesos-git > > > Description > ------- > > Add basic stub for dynamic slave attributes > > > Diffs > ----- > > src/common/attributes.hpp 0a043d5 > src/common/attributes.cpp aab114e > src/master/master.hpp c9f989a > src/master/master.cpp 2508b38 > src/slave/slave.hpp 9d4607e > src/slave/slave.cpp 5c76dd1 > > Diff: https://reviews.apache.org/r/25111/diff/ > > > Testing > ------- > > This is currently a work in progress, (WIP) > > > Thanks, > > Patrick Reilly > >
