Michael Ho has posted comments on this change.

Change subject: IMPALA-3223: Removal of non-toolchain builds.
......................................................................


Patch Set 2:

(3 comments)

Found some more clean up opportunities since patch set 2. Included them in 
patch set 3.

http://gerrit.cloudera.org:8080/#/c/3259/2/CMakeLists.txt
File CMakeLists.txt:

Line 78: set(ENV{BOOST_ROOT} 
"$ENV{IMPALA_TOOLCHAIN}/boost-$ENV{IMPALA_BOOST_VERSION}")
> Does this need to be in the CMakeLists.txt? It would be nice if all environ
Done


http://gerrit.cloudera.org:8080/#/c/3259/2/bin/impala-config.sh
File bin/impala-config.sh:

Line 63: "\
       :        "
> missing a space between these words
The line continuation adds a space automatically.

$ export IMPALA_TOOLCHAIN=""
$ . ./bin/impala-config.sh
IMPALA_TOOLCHAIN must be specified. Please set it to a valid location or leave 
it unset.


Line 61: # The default value of IMPALA_TOOLCHAIN is set above.
       : if [ -z $IMPALA_TOOLCHAIN ]; then
       :   echo "IMPALA_TOOLCHAIN must be specified. Please set it to a valid 
location or"\
       :        "leave it unset."
       :   return 1
       : fi
> Can you move this closer to l42 or move that down here?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42b60e99fb9caf1294be7ab242856ca3b9a5ab73
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-HasComments: Yes

Reply via email to