> On Oct. 21, 2014, 5:23 p.m., Vinod Kone wrote: > > src/common/slaveinfo_utils.hpp, line 32 > > <https://reviews.apache.org/r/25525/diff/8-11/?file=717600#file717600line32> > > > > Actually I think this would be better as "Option<Error> compatible()" > > instead of "Try<bool> isCompatible()", similar to how we do validateTask() > > in master.cpp. For places where you need to check whether nothing has > > changed, an equality check can be used.
Doing a seperate equality operation about doubles the computation at most callsites without needing to. It also complicates the callsite. Currently 2 out of 3 callers of the function need to check for the equality case. So they would have to gain more code to first check if the optional is set or not, if it is propogate up the error. And if not, then check for equality. Changed the name to compatible(). - Cody ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review57511 ----------------------------------------------------------- On Oct. 21, 2014, 11:55 p.m., Cody Maloney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25525/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2014, 11:55 p.m.) > > > Review request for mesos, Adam B and Vinod Kone. > > > Bugs: MESOS-1739 > https://issues.apache.org/jira/browse/MESOS-1739 > > > Repository: mesos-git > > > Description > ------- > > Allows attributes and resources to be set to a superset of what they were > previously on a slave restart. > > Incorporates all comments from: > https://issues.apache.org/jira/browse/MESOS-1739 > and the former review request: > https://reviews.apache.org/r/25111/ > > > Diffs > ----- > > src/Makefile.am c44a9ad47d6e1262949b9049f4ae25b049440d99 > src/common/attributes.hpp 0a043d5b5dca804c6dd215cabd2704f24df71a33 > src/common/attributes.cpp aab114e1a5932e3f218b850e1afc7f2ef0f10e21 > src/common/slaveinfo_utils.hpp PRE-CREATION > src/common/slaveinfo_utils.cpp PRE-CREATION > src/master/allocator.hpp 02d20d0cc802805bc702891306aa42894531b223 > src/master/hierarchical_allocator_process.hpp > 31dfb2cc86e2e74da43f05fbada10976ce65f3e4 > src/master/master.hpp 18898e9e1897b9d8cecd10d0ef7f25540cc9916d > src/master/master.cpp be910d9f02405e25e5ad6ae3bf13d770a0c2b417 > src/slave/slave.cpp 7b5474ae898f369fc55670cf1dbbee684bc0ae4b > src/tests/attributes_tests.cpp 240a8cac18ac12178cf73e8eeb88bd50e3fcc03b > src/tests/mesos.hpp e40575c6d330cfa3fbfad2efcf601418c413b992 > src/tests/slave_tests.cpp 759670ad91c2f8afbd4b9dcb2f367f50482358e9 > > Diff: https://reviews.apache.org/r/25525/diff/ > > > Testing > ------- > > make check on localhost > > > Thanks, > > Cody Maloney > >
