You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Diana Arroyo <da...@us.ibm.com> on 2016/01/21 17:58:23 UTC
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/
-----------------------------------------------------------
(Updated Jan. 21, 2016, 4:58 p.m.)
Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
Changes
-------
Addressed latest review comments.
Summary (updated)
-----------------
CMake: Added CMake file for agent executable build.
Bugs: MESOS-3843
https://issues.apache.org/jira/browse/MESOS-3843
Repository: mesos
Description
-------
Added CMake file for agent executable build.
Diffs (updated)
-----
src/slave/CMakeLists.txt PRE-CREATION
src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
Diff: https://reviews.apache.org/r/41092/diff/
Testing
-------
Tested on Ubuntu and OSX.
Thanks,
Diana Arroyo
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/#review116371
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40951, 41090, 41092]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 26, 2016, 1:54 p.m., Diana Arroyo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2016, 1:54 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added CMake file for agent executable build.
>
>
> Diffs
> -----
>
> src/slave/CMakeLists.txt PRE-CREATION
> src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
>
> Diff: https://reviews.apache.org/r/41092/diff/
>
>
> Testing
> -------
>
> Tested on Ubuntu and OSX.
>
>
> Thanks,
>
> Diana Arroyo
>
>
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/#review121005
-----------------------------------------------------------
src/slave/CMakeLists.txt (line 27)
<https://reviews.apache.org/r/41092/#comment182584>
I liked what you did with the master -- there should be only a `main.cpp` file here, so what do you think of just making this `add_executable(${AGENT_TARGET} main.cpp)`?
src/slave/CMakeLists.txt (line 33)
<https://reviews.apache.org/r/41092/#comment182587>
Same comment as the master review -- I recommend putting the `*_TARGET`s inside a single variable inside `SlaveConfigure.cmake`, and simply calling `add_dependencies(${AGENT_TARGET} ${AGENT_DEPENDENCIES})` or whatever. This should be very similar to the slave.
src/slave/CMakeLists.txt (line 37)
<https://reviews.apache.org/r/41092/#comment182588>
See comment above and comment in the master review for my recommendations here -- thinking it woudl be better to just put this into a single variable in `SlaveConfigure.cmake`.
- Alex Clemmer
On Feb. 13, 2016, 4:23 a.m., Diana Arroyo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2016, 4:23 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added CMake file for agent executable build.
>
>
> Diffs
> -----
>
> src/slave/CMakeLists.txt PRE-CREATION
> src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
>
> Diff: https://reviews.apache.org/r/41092/diff/
>
>
> Testing
> -------
>
> Tested on Ubuntu and OSX.
>
>
> Thanks,
>
> Diana Arroyo
>
>
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/#review119141
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40951, 41090, 41092]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 13, 2016, 4:23 a.m., Diana Arroyo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2016, 4:23 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added CMake file for agent executable build.
>
>
> Diffs
> -----
>
> src/slave/CMakeLists.txt PRE-CREATION
> src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
>
> Diff: https://reviews.apache.org/r/41092/diff/
>
>
> Testing
> -------
>
> Tested on Ubuntu and OSX.
>
>
> Thanks,
>
> Diana Arroyo
>
>
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
Posted by Diana Arroyo <da...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/
-----------------------------------------------------------
(Updated Feb. 13, 2016, 4:23 a.m.)
Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
Changes
-------
Rebase per Alex's request.
Bugs: MESOS-3843
https://issues.apache.org/jira/browse/MESOS-3843
Repository: mesos
Description
-------
Added CMake file for agent executable build.
Diffs (updated)
-----
src/slave/CMakeLists.txt PRE-CREATION
src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
Diff: https://reviews.apache.org/r/41092/diff/
Testing
-------
Tested on Ubuntu and OSX.
Thanks,
Diana Arroyo
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
Posted by Diana Arroyo <da...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/
-----------------------------------------------------------
(Updated Jan. 26, 2016, 1:54 p.m.)
Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
Changes
-------
Cleanup code.
Bugs: MESOS-3843
https://issues.apache.org/jira/browse/MESOS-3843
Repository: mesos
Description
-------
Added CMake file for agent executable build.
Diffs (updated)
-----
src/slave/CMakeLists.txt PRE-CREATION
src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
Diff: https://reviews.apache.org/r/41092/diff/
Testing
-------
Tested on Ubuntu and OSX.
Thanks,
Diana Arroyo
Re: Review Request 41092: CMake: Added CMake file for agent
executable build.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41092/#review115685
-----------------------------------------------------------
I was expecting a `mesos-slave` executable, but I don't see any `add_executable` (or something that actually generates an executable) in this patch. Was that intentional?
src/slave/CMakeLists.txt (lines 17 - 19)
<https://reviews.apache.org/r/41092/#comment176709>
Clean this up?
src/slave/CMakeLists.txt (line 42)
<https://reviews.apache.org/r/41092/#comment176701>
Nit: Pre-commit hooks complain about this line. (Delete it)
- Joseph Wu
On Jan. 21, 2016, 8:58 a.m., Diana Arroyo wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41092/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 8:58 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-3843
> https://issues.apache.org/jira/browse/MESOS-3843
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added CMake file for agent executable build.
>
>
> Diffs
> -----
>
> src/slave/CMakeLists.txt PRE-CREATION
> src/slave/cmake/SlaveConfigure.cmake fbdfdaa27fbd8c7429861eea5baf401a221f748b
>
> Diff: https://reviews.apache.org/r/41092/diff/
>
>
> Testing
> -------
>
> Tested on Ubuntu and OSX.
>
>
> Thanks,
>
> Diana Arroyo
>
>