Re: Review Request 18370: Refactor Cluster::Master::start methods

[Vinod Kone <vi...@gmail.com>]

> 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.

How about MasterInfo (if it doesn't conflict with the protobuf in mesos.proto) or MasterData?


- Vinod


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


On Feb. 26, 2014, 10:19 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18370/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2014, 10:19 p.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
> 
>