You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Marco Massenzio <ma...@mesosphere.io> on 2015/05/13 22:12:25 UTC
Re: Review Request 33376: MESOS-2633 Moved struct Framework methods
to their own implementation class.
-----------------------------------------------------------
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
Re: Review Request 33376: MESOS-2633 Moved struct Framework methods
to their own implementation class.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
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
>
>