You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/04/14 02:03:44 UTC

Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
-------

This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
and consolidates platform-specific versions into `Versions.cmake`.


Diffs
-----

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake c60652688a23f8628f133b7890ff39e38fc8ae94 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 


Diff: https://reviews.apache.org/r/58447/diff/1/


Testing
-------


Thanks,

Andrew Schwartzmeyer


Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58447/#review173016
-----------------------------------------------------------


Ship it!




I'm going to make a couple minor whitespace additions (to make things consistent within the files).  But otherwise LGTM.


3rdparty/CMakeLists.txt
Line 46 (original), 26-27 (patched)
<https://reviews.apache.org/r/58447/#comment246089>

    Whitespace things like: 2 spaces before each of these sections.



3rdparty/CMakeLists.txt
Lines 54-55 (original), 51-52 (patched)
<https://reviews.apache.org/r/58447/#comment246090>

    And one newline before comments like this.


- Joseph Wu


On April 13, 2017, 7:03 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> -----------------------------------------------------------
> 
> (Updated April 13, 2017, 7:03 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.

> On April 18, 2017, 8:25 p.m., Jeff Coffler wrote:
> > I'd like to understand the tests that you ran before you posted the review. What, specifically, did you test on to insure that nothing was broken by this change?

Please see the last patch in the chain for tests.


- Andrew


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


On April 14, 2017, 2:03 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> -----------------------------------------------------------
> 
> (Updated April 14, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58447/#review172260
-----------------------------------------------------------



I'd like to understand the tests that you ran before you posted the review. What, specifically, did you test on to insure that nothing was broken by this change?

- Jeff Coffler


On April 14, 2017, 2:03 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> -----------------------------------------------------------
> 
> (Updated April 14, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58447/#review172812
-----------------------------------------------------------


Ship it!




Ship It!

- Jeff Coffler


On April 14, 2017, 2:03 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> -----------------------------------------------------------
> 
> (Updated April 14, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58447/#review171979
-----------------------------------------------------------



This pays back a lot of technical debt and confusing build cruft (where the version set in Versions.cmake was not necessarily the version used).

- Andrew Schwartzmeyer


On April 14, 2017, 2:03 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> -----------------------------------------------------------
> 
> (Updated April 14, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>