impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [native-toolchain-CR] Add a script to build Kudu using existing toolchain artifacts
Date Fri, 03 Mar 2017 00:14:00 GMT
Matthew Jacobs has posted comments on this change.

Change subject: Add a script to build Kudu using existing toolchain artifacts
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6167/2/build-kudu-single.sh
File build-kudu-single.sh:

Line 1: #!/usr/bin/env bash
> Doesn't matter too much but build-kudu-only seems clearer to me
Done


Line 15: 
> Can you add a short comment up the top explaining how to use it and what en
Done


Line 35: function download_toolchain_dependency() {
> Does this download all versions of the package?
Whatever is in that directory, I suppose that would be unnecessary if the toolchain build
was 'historical', though I'm pretty sure we never do that. (In fact I was thinking we should
remove 'historical' anyway.)


Line 110:   pkg_name=${dir%*/}
> Can we call this something instead of pkg_name? Like pkg_string? pkg_name u
Done


http://gerrit.cloudera.org:8080/#/c/6167/2/init-compiler.sh
File init-compiler.sh:

PS2, Line 20:  
> trailing space
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I237580e1545033467a92285ca8bb8db1cf8c804e
Gerrit-PatchSet: 2
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message