Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Joris Van Remoortere]

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

Ship it!


This is great Alex!

- Joris Van Remoortere


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Artem Harutyunyan]

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


LGTM.

- Artem Harutyunyan


On Sept. 20, 2015, 8:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 20, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[haosdent huang]

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

Ship it!


Ship It!

- haosdent huang


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Alex Clemmer]

> On Sept. 22, 2015, 6:32 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/cmake/ProcessConfigure.cmake, lines 59-65
> > 
> >
> > What do you think about moving this `if` block into `Versions.cmake`?
> > 
> > Pro: Versions are kept in one place.  If we unilaterally update to glog 
> > 0.3.4, we'll only need to update it in one place.
> > 
> > Con: Logic in the version file, which we don't really have a precedent 
> > for.

I think it would be best to keep it in `ProcessConfigure.cmake`. The reason is 
that then anyone who uses libprocess need only `include(ProcessConfigure)` and 
the third-party build logic is generated seemlessly for them. If you put this 
in `Versions.cmake`, then you'd have to `include` both to fully configure and 
use libprocess, and you'd have to move `Versions.cmake` back into the 
`libprocess` directory.

Besides that, we're going to upgrade glog to 0.3.5 at some point anyway, and in 
that case, the conditional will disappear, and it will stop making much sense 
to have it in `Versions.cmake`.


- Alex


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


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Joseph Wu]

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

Ship it!



3rdparty/libprocess/cmake/ProcessConfigure.cmake (lines 59 - 65)


What do you think about moving this `if` block into `Versions.cmake`?

Pro: Versions are kept in one place.  If we unilaterally update to glog 
0.3.4, we'll only need to update it in one place.

Con: Logic in the version file, which we don't really have a precedent for.


- Joseph Wu


On Sept. 20, 2015, 8:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 20, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Alex Clemmer]

> On Sept. 21, 2015, 6:15 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake, line 40
> > 
> >
> > Did you miss this one?  Or was it left out intentionally?

We don't seem to be using it yet, so I left it out.


- Alex


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


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Joseph Wu]

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



3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake (line 40)


Did you miss this one?  Or was it left out intentionally?


- Joseph Wu


On Sept. 20, 2015, 8:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 20, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 
38541, 38542]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>