mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.
Date Thu, 14 May 2015 00:27:19 GMT

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


Per the offline discussion, how about we create a master/framework.hpp (and master/slave.hpp
later), much like we did for master/metrics.hpp? Having definitions in master.hpp that are
defined in framework.cpp is a bit unintuitive (I've seen a number of people get confused about
this approach in master/http.cpp).

Note that originally a master/metrics.cpp file was added on the assumption that it would speed
up build times, which likely didn't hold. Since you didn't find a compile time decrease from
the current approach, I'd suggest just keeping all the code together in a master/framework.hpp
header. Note also that this lets you forward declare '`Framework`'.

- Ben Mahler


On May 13, 2015, 8:12 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33376/
> -----------------------------------------------------------
> 
> (Updated May 13, 2015, 8:12 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2633
>     https://issues.apache.org/jira/browse/MESOS-2633
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Created new file framework.cpp containing all the methods' implementations for the `Framework`
class (declared in master/master.hpp)
> 
> Declared `operator ==` for Task in type_utils.hpp 
> (it was implemented before, but not declared in the header file);
> 
> Refactored all the LOG(WARNING) to a single utility method.
> 
> 
> Diffs
> -----
> 
>   include/mesos/type_utils.hpp 044637481e5405d4d6f61653a9f9386edd191deb 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/framework.cpp PRE-CREATION 
>   src/master/master.hpp 49ee050ca4d2b2c5f75ce864fcf6ae703dfdeadd 
> 
> Diff: https://reviews.apache.org/r/33376/diff/
> 
> 
> Testing
> -------
> 
> All tests (make check) pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message