----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66008/#review199176 -----------------------------------------------------------
cmake/CompilationConfigure.cmake Lines 220 (patched) <https://reviews.apache.org/r/66008/#comment279470> Can we at least add a `TODO` here for what was advertised in the commit message, > This also replaces the use of string(APPEND CMAKE_CXX_FLAGS) with the canonical command add_compile_options (though setting these on a per-target basis would be even better, it's a separate issue). Ideally we would reference a ticket. No need to add this everywhere below, but this one sticks out especially since it says `GLOBAL` :/ cmake/CompilationConfigure.cmake Lines 224 (patched) <https://reviews.apache.org/r/66008/#comment279471> Nit: let's put these each flag on a separate line already now, it'll be much nicer to evolve over time. cmake/CompilationConfigure.cmake Lines 226 (patched) <https://reviews.apache.org/r/66008/#comment279472> We definitely do not want to include `-Weverything` ever. It is intended for testing diagnostics by upstream developers, but would warn about issues we would not want to address (e.g., struct packing). You probably meant `-Wextra` like in the `GNU` case. src/CMakeLists.txt Lines 485 (patched) <https://reviews.apache.org/r/66008/#comment279473> We should add a cmake configuration flag to disable `-Werror`. We have an equivalent configure flag `--disable-werror` since it is extremely unlikely that we'll support every possible setup (e.g., bleeding edge compilers, packaging setups). Let's give users a way to opt out. - Benjamin Bannier On March 9, 2018, 11:38 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66008/ > ----------------------------------------------------------- > > (Updated March 9, 2018, 11:38 p.m.) > > > Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John > Kordich, Joseph Wu, and Michael Park. > > > Bugs: MESOS-8658 > https://issues.apache.org/jira/browse/MESOS-8658 > > > Repository: mesos > > > Description > ------- > > We had previously been using the default sets of warnings, but now we > use the same warnings as on Autotools. This also means that we can > safely turn on the treat-warnings-as-errors (at least for the Mesos > code). However, doing so requires that we disable two common > possible-loss-of-data warnings on Windows that are not part of the > GNU/Clang default warnings. > > This also replaces the use of `string(APPEND CMAKE_CXX_FLAGS)` with > the canonical command `add_compile_options` (though setting these on a > per-target basis would be even better, it's a separate issue). > > > Diffs > ----- > > cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 > src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef > > > Diff: https://reviews.apache.org/r/66008/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrew Schwartzmeyer > >
