Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12162 )
Change subject: Support CXXFLAGS ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/12162/1/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12162/1/CMakeLists.txt@125 PS1, Line 125: # compiler flags that are common across debug/release builds : # -msse4.2: Enable sse4.2 compiler intrinsics. : set(CXX_COMMON_FLAGS "-msse4.2") : # -Wall: Enable all warnings. > That makes a lot of sense. I did the simplest possible thing in the next PS The order of the flags does matter, and we should probably make sure the user's flags (i.e. the initial value of CMAKE_CXX_FLAGS) comes _last_, so that it takes precedence over all of Kudu's flags. That said, it looks like we append to CMAKE_CXX_FLAGS all over the place here, so don't worry about that (unless you really want to address it). http://gerrit.cloudera.org:8080/#/c/12162/2/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12162/2/CMakeLists.txt@210 PS2, Line 210: # Append to compile flags based on the build type. Could you add a comment here noting that it's important not to clobber CMAKE_CXX_FLAGS because it may contain flags specified by the user? -- To view, visit http://gerrit.cloudera.org:8080/12162 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9dcad5ccb4fc4c081d96eaa40957175731c7a9e Gerrit-Change-Number: 12162 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sat, 05 Jan 2019 00:07:05 +0000 Gerrit-HasComments: Yes
