You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2017/09/03 01:44:24 UTC

Re: Review Request 61365: Added `get_byproducts()` function to fix Ninja generator.

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


Ship it!




There's a bit of inconsistency with regards to capitalization of helper methods.  After this chain is committed, I'm going to sweep through the CMake code again and make sure all helper methods are using SCREAMING_SNAKE_CASE.  This helps denote that a method is not a built-in function, but something we've defined.

- Joseph Wu


On Aug. 9, 2017, 7:53 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61365/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2017, 7:53 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-3576, MESOS-5656 and MESOS-6794
>     https://issues.apache.org/jira/browse/MESOS-3576
>     https://issues.apache.org/jira/browse/MESOS-5656
>     https://issues.apache.org/jira/browse/MESOS-6794
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake also supports Ninja as a generator, but Ninja requires that each
> use of `ExternalProject_Add` specifies its `BUILD_BYPRODUCTS`.
> Interestingly, it doesn't care about interface libraries, and
> furthermore, it can't derive the dependency from other
> information (despite it being available).
> 
> Review: https://reviews.apache.org/r/61365
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
> 
> 
> Diff: https://reviews.apache.org/r/61365/diff/2/
> 
> 
> Testing
> -------
> 
> This is the last patch in a long chain of CMake refactoring.
> 
> This has been tested on Windows 10, CentOS 7, and macOS. While I've worked on it extensively, it could still use yet more testing by others to ensure all configurations and environments still work properly.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>