> On Feb. 26, 2014, 7:55 p.m., Vinod Kone wrote:
> > src/tests/cluster.hpp, line 108
> > <https://reviews.apache.org/r/18370/diff/3/?file=503678#file503678line108>
> >
> >     Why this name change? I rather liked the former.
> 
> Charlie Carson wrote:
>     this was based on feedback from Dominic.  I'm not particularly fond of 
> OwnedDependents, but it's more accurate than Master.  Master is horribly 
> overloaded - it's a private internal Master struct which itself contains a 
> mesos::internal::Master * and is inside of a class named Masters.
> 
> Vinod Kone wrote:
>     How about MasterInfo (if it doesn't conflict with the protobuf in 
> mesos.proto) or MasterData?
> 
> Charlie Carson wrote:
>     I'd try MasterInfo and see if that works.

Both MasterInfo and Master are indeed overloaded, but if we are going to make 
this change let's be consistent with the Slave struct in this file too please. 
And honestly, since everything is overloaded here I'd prefer to just see this 
part of the change reverted (or a different name all together).


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18370/#review35561
-----------------------------------------------------------


On March 1, 2014, 1:15 a.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18370/
> -----------------------------------------------------------
> 
> (Updated March 1, 2014, 1:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1024
>     https://issues.apache.org/jira/browse/MESOS-1024
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
>     Refactor Cluster::Master::start methods
> 
>     There are currently two overloads of the Cluster::Masters::start
>     function.  One takes a argument of AllocatorProcess* and the other
>     does not.  The AllocatorProcess* overload serves two purposes:
>     1) it allows an alternative implementation of AllocatorProcess to be
>     passed (i.e. a mock)
>     2) it changes the destruction timing so that the passed in argument
>     can outlive the master.
> 
>     Beyond that, the two functions are identical.   This changes the
>     parameter to be Option<AllocatorProcess*> and allows all the logic
>     to be in one method.  The old function exists for back-compat but
>     now simply forward by passing in None() to the other function.
> 
>     This is for two purposes:
>     1) reduce code duplication
>     2) position the code so that we can also optionally pass in a mock
>        repaier.
> 
>     Review: https://reviews.apache.org/r/18370
> 
> 
> Diffs
> -----
> 
>   src/tests/cluster.hpp d1bf680ed3f6a0fb16af85a21409d653d44522ca 
> 
> Diff: https://reviews.apache.org/r/18370/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>

Reply via email to