Re: Review Request 41092: CMake: Added CMake file for agent executable build.

[Joris Van Remoortere]

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


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.

[Alex Clemmer]

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




src/slave/CMakeLists.txt (line 27)


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)


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)


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.

[Diana Arroyo]

---
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.

[Mesos ReviewBot]

---
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.

[Diana Arroyo]

---
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.

[Mesos ReviewBot]

---
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.

[Diana Arroyo]

---
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.

[Joseph Wu]

---
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)


Clean this up?



src/slave/CMakeLists.txt (line 42)


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
> 
>