Tim Armstrong has posted comments on this change.

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


Patch Set 2:

(5 comments)

Looks pretty good, just had some minor 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


Line 15: 
Can you add a short comment up the top explaining how to use it and what 
environment variables influence it.


Line 35: function download_toolchain_dependency() {
Does this download all versions of the package?


Line 110:   pkg_name=${dir%*/}
Can we call this something instead of pkg_name? Like pkg_string? pkg_name 
usually means the name without the version in these scripts.


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

PS2, Line 20:  
trailing space


-- 
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 <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to