impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Add all build targets to CMake and speed up builds
Date Mon, 24 Oct 2016 20:57:22 GMT
Tim Armstrong has posted comments on this change.

Change subject: Add all build targets to CMake and speed up builds
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4790/1/bin/make_impala.sh
File bin/make_impala.sh:

Line 90:       echo "[-fe_only] : Builds fe only."
> The reason for this flag is so that make_impala.sh still builds the backend
A lot of people depend on make_impala.sh only building the backend unfortunately (I know of
several jenkins jobs). Doesn't seem worth the pain, especially since buildall.sh already was
a standard way to build everything.


PS1, Line 92: ."
> cand benchmarks
Done


PS1, Line 96: like
> What else?
There used to be a test tarball. We might want to produce a source tarball at some point.
Mainly just didn't want to add a new flag every time we added some new build artifact.


PS1, Line 174: exit
> This line can be omitted
Done


http://gerrit.cloudera.org:8080/#/c/4790/1/buildall.sh
File buildall.sh:

PS1, Line 305: all
> Should the meaning of "all" be put into make_impala.sh?
I think it makes more sense to do that at the CMake level. It already builds most of these
things when run with no args, but I added in the remaining ones that were not part of ALL
(cscope and the tarballs).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message