> On Sept. 11, 2015, 7:25 p.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 166
> > <https://reviews.apache.org/r/37502/diff/1/?file=1041030#file1041030line166>
> >
> >     So, what's the rationale for using `${CMAKE_CXX_FLAGS}` here? It seems 
> > to me that we only want the flags that GTest and GMock need. Is that right? 
> > What am I missing?
> >     
> >     Additionally, do we wonly want to set `GTEST_LANG_CXX11` in the case 
> > that we're compiling on Clang? If so, why?
> 
> Alex Clemmer wrote:
>     To add specific recommendations here, assuming the issues I've raised are 
> correct:
>     
>     1. I recommend we remove of the `${CMAKE_CXX_FLAGS}` from the 
> `GMOCK_CONFIG_CXXFLAGS` variable.
>     2. I recommend we remove `-DGTEST_USE_OWN_TR1_TUPLE` anywhere we are 
> passing it in (including the autotools build system), as it seems GMock 1.7 
> makes it irrelevant.
>     3. I recommend we move `-DGTEST_LANG_CXX11` out of the `if (APPLE)` tags, 
> as there does not seem to be rationale about why it should only be compiled 
> as C++11 on Appleclang. (If this is not correct, I recommend we add a comment 
> explaining why it is not correct.)

> ${CMAKE_CXX_FLAGS}

${CMAKE_CXX_FLAGS} is used for pass CXX_FLAGS from outsite CMakeLists.txt. Like 
this one in MesosConfigure.cmake:
```
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
```

> -DGTEST_LANG_CXX11

I think it still necessary for clang to use c++11 to compile gtest. 
https://github.com/google/googlemock/blob/release-1.7.0/include/gmock/gmock-matchers.h#L56
 and  
https://github.com/google/googletest/blob/release-1.7.0/include/gtest/internal/gtest-port.h#L272

> -DGTEST_USE_OWN_TR1_TUPLE

I think we want to make sure the consistency (always use google tuple) in 
different environments before. We could remove it in autotools and cmake 
because you metion it has already set intelligently.


- haosdent


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


On Sept. 4, 2015, 4:25 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37502/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2015, 4:25 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, and Joseph Wu.
> 
> 
> Bugs: MESOS-3270
>     https://issues.apache.org/jira/browse/MESOS-3270
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add CMAKE_CXX_FLAGS to GMOCK_CONFIG_CMD in CMake.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 997cc0d0e316e316136d4746e50e9e292a82b36b 
> 
> Diff: https://reviews.apache.org/r/37502/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to