mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 53764: CMake: Added a target for the default executor.
Date Mon, 28 Nov 2016 23:24:06 GMT


> On Nov. 28, 2016, 11:13 a.m., Alex Clemmer wrote:
> > src/launcher/CMakeLists.txt, line 48
> > <https://reviews.apache.org/r/53764/diff/1/?file=1563927#file1563927line48>
> >
> >     Are we deleting this line because this is included also in the `src/` directory?
Do you think there is any benefit in having submodules include everything they need to build,
themselves, rather than depending on the `src/` directory to set it up?

There isn't any real benefit, unless we greatly rearrange the codebase.

With our current codebase, most files implicitly expect to have access to *internal* headers
from the top level of the source.  This means we don't have a very strong separation of components
in the codebase (the agent imports master things; the master imports agent things; etc). 
But at the same time, it also makes development simpler, as we don't have to mess with the
build system to add extra includes.

---

As for why I've deleted this line, the CMake "environment" is inherited by children files,
but not the other way around.  By importing in multiple places, we are just adding code bloat,
with no real change in functionality.  It would be a different story if each `include_directories`
line had something besides `${AGENT_INCLUDE_DIRS}` (which is defined globally).


- Joseph


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


On Nov. 14, 2016, 7:15 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53764/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5792
>     https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This executor has support for TaskGroups and will eventually take
> precedence over the default command executor.
> 
> 
> Diffs
> -----
> 
>   cmake/MesosConfigure.cmake de04389b016b22bdff69e94d37fad4eedbda5874 
>   src/launcher/CMakeLists.txt 3137ea1479970c7ea46e6595536d481002fc585c 
> 
> Diff: https://reviews.apache.org/r/53764/diff/
> 
> 
> Testing
> -------
> 
> cmake ..
> make
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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