> On May 6, 2015, 10:39 p.m., Vinod Kone wrote: > > include/mesos/master/allocator.hpp, lines 54-55 > > <https://reviews.apache.org/r/33513/diff/4/?file=951247#file951247line54> > > > > Instead of empty string, make the argument Option<string>? > > Alexander Rukletsov wrote: > I think a common pattern in such cases is to use a constant like > `DEFAULT_XXX` and not `Option<string>`, e.g. `MasterDetector`, > `Authenticator`, `Authenticatee`. > > Vinod Kone wrote: > if there is a DEFAULT_* constant, isn't that always be passed to the > create() to get the default module? Why would one need to pass an empty > string in that case?
We do not pass empty strings any more : ). There is a `DEFAULT_*` constant instead. I have updated the comments, empty string is not mentioned anywhere now. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33513/#review82748 ----------------------------------------------------------- On May 8, 2015, 11:47 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33513/ > ----------------------------------------------------------- > > (Updated May 8, 2015, 11:47 p.m.) > > > Review request for mesos, Kapil Arya, Niklas Nielsen, and Till Toenshoff. > > > Bugs: MESOS-2597 > https://issues.apache.org/jira/browse/MESOS-2597 > > > Repository: mesos > > > Description > ------- > > With the support of allocator modules, it should be possible to select and > instantiate an allocator based on its name. > > > Diffs > ----- > > include/mesos/master/allocator.hpp e821bf33536ab70cc2be45b9239c089e251f1f24 > src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b > src/master/allocator/allocator.cpp PRE-CREATION > src/master/allocator/mesos/allocator.hpp > 2681af5ff265b84c5174de3ad459292e006dcf97 > src/master/allocator/mesos/hierarchical.hpp > 09adced9d8712b3eeda885d598443791186890db > src/master/constants.hpp c386eab3973363e64c113f7e84452456fdd3393c > src/master/constants.cpp 9ee17e9ad884da98cabfdfe316cb4a831de193b3 > src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd > > Diff: https://reviews.apache.org/r/33513/diff/ > > > Testing > ------- > > make check (Mac OS 10.9.5, CentOS 7.0) > > > Thanks, > > Alexander Rukletsov > >