----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44001/#review121009 -----------------------------------------------------------
src/master/cmake/MasterConfigure.cmake (line 49) <https://reviews.apache.org/r/44001/#comment182590> Which of these do we actually need for the master? It looks like we're copying this directly from the stuff in `SlaveConfigure.cmake`, is that right? If so, two notes: (1) The `AGENT_INCLUDE_DIRS`, `AGENT_DEPENDENCIES`, and so on, are actually misnomers -- it should really be called `MESOS_INCLUDE_DIRS`, _etc_., because it is building libmesos, not the slave. (2) Therefore, I recommend renaming those variables to be `MESOS_*` instead of `AGENT_*`, and then just passing the `MESOS_INCLUDE_DIRS` into the `MASTER_INCLUDE_DIRS` instead of copying them everywhere. Possibly this involves moving those definitions to `MesosConfigure.cmake` instead, as the libmesos configuration really doesn't belong in `SlaveConfigure.cmake`. My rationale for point 2 here, is that we are just copying around the smae include paths in a bunch of places now, and this is bad. Every time we add a directory, we'll need to add it to two places. Why not just add it to the libmesos configuration once, and have both other things consume that configuration instead? Thoughts? - Alex Clemmer On Feb. 25, 2016, 8:53 p.m., Diana Arroyo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44001/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2016, 8:53 p.m.) > > > Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van > Remoortere, and Joseph Wu. > > > Bugs: MESOS-4773 > https://issues.apache.org/jira/browse/MESOS-4773 > > > Repository: mesos > > > Description > ------- > > CMake: Add MasterConfigure for master executable build. > > > Diffs > ----- > > src/master/cmake/MasterConfigure.cmake PRE-CREATION > > Diff: https://reviews.apache.org/r/44001/diff/ > > > Testing > ------- > > Tested on Ubuntu. > > > Thanks, > > Diana Arroyo > >
