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



Could you please link a ticket to this?

This is a weird way to "modularize". While we depend on the header file we do 
not properly depend on the library providing most of its symbols in the builds 
setup. I'd prefer that if we depend on some header we have a matching 
dependency in the build setup to avoid nasty surprises (if we are lucky only at 
link time, but maybe even ODR violations if the stars align).

To accomplish this we could e.g., extract the common symbols into their own 
component (headers + library), and update the consumers with a dependency on 
that. To gauge whether that's a good approach having a ticket with a problem 
description at hand would be very helpful.

- Benjamin Bannier


On Jan. 15, 2019, 11:27 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69755/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2019, 11:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This removes the need to link 'src/containerizer/mesos/launch.cpp'
> with the components (MesosContainerizer actor and Command Executor
> binary) that use the 'mesos-containerizer' helper binary.
> 
> 
> Diffs
> -----
> 
>   src/launcher/executor.cpp f962e800f23d5582b1bc04a263253893492a5054 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 5016f2e9f0651abcb0a5f364e8eace458f2edeae 
>   src/slave/containerizer/mesos/launch.hpp 
> 0a6394d56321948ad760ac69c05456319a254842 
>   src/slave/containerizer/mesos/launch.cpp 
> 2f1c9e7a8748c9d7eab25bc8567ca68308e680f9 
>   src/tests/containerizer/port_mapping_tests.cpp 
> b511016fa7cd80d2ffee1747e5e463cc1b2d56bb 
> 
> 
> Diff: https://reviews.apache.org/r/69755/diff/1/
> 
> 
> Testing
> -------
> 
> cmake --build .
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to