Lars Volker has posted comments on this change.

Change subject: Add -release switch to buildall.sh help, change coverage 
options.
......................................................................


Patch Set 5:

(5 comments)

Sorry, I forgot to hit reply.

http://gerrit.cloudera.org:8080/#/c/2043/5/buildall.sh
File buildall.sh:

Line 95:       TARGET_BUILD_TYPE=Release
> We should consider renaming TARGET_BUILD_TYPE to CMAKE_BUILD_TYPE because t
Done


Line 100:     -codecoverage_debug)
> Can we remove these for simplicity?
Done


Line 203: # Set correct TARGET_BUILD_TYPE.
> Maybe "Adjust CMAKE_BUILD_TYPE for ASAN and code coverage, if necessary."
Done


Line 212:     *)
> I think we can remove this or alternatively check the CMAKE_BUILD_TYPE outs
Done


Line 218: if [[ ${BUILD_ASAN} -eq 1 ]]; then
> Also add a check for BUILD_ASAN=1 and BUILD_COVERAGE=1 which we don't suppo
BUILD_COVERAGE=1 will set the CMAKE_BUILD_TYPE to one of 
CODE_COVERAGE_{DEBUG,RELEASE} and the check in line 219 will print an error 
including the wrong type: "Address sanitizer build not supported for build 
type: CODE_COVERAGE_DEBUG". Would that be sufficient or should we have a more 
explicit check? I added a comment explaining how we also catch that case with 
the check.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id69791264cb2d9e0ffe96a7ac5aabc34a553a7be
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: Yes

Reply via email to