Tim Armstrong has posted comments on this change.

Change subject: IMPALA-2949: enable -Werror for debug builds
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2420/1/be/CMakeLists.txt
File be/CMakeLists.txt:

Line 77:   #   Only enable for debug builds because this is what we test in 
pre-commit tests.
> Why not also enable this for asan and release builds?
I think compile errors should be pretty easy to debug.

The other approach would be to add a cmake option for Werror and enable it 
per-job, but I think this adds another variable where local builds can succeed 
but pre-commits can fail.

I'm in favour of enabling it for ASAN and release builds once we test that in 
precommit testing. I didn't want to add another failure mode where precommit 
tests succeed but another build totally breaks, I think this change is more of 
an incremental step forward. There are also additional unfixed warnings in 
those builds.


-- 
To view, visit http://gerrit.cloudera.org:8080/2420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I04846b45738be37d372927992ff666801a3b29ad
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to