Matthew Jacobs has posted comments on this change. Change subject: Add Kudu using parcels ......................................................................
Patch Set 1: (4 comments) Your bash skills are unparalleled. This isn't a full review, I just got a chance to look quickly. I have to get to an appointment but will get back to this later. http://gerrit.cloudera.org:8080/#/c/2627/1/functions.sh File functions.sh: Line 57: -$(basename $URL) What does this mean? (I don't know how to search Google for this.) http://gerrit.cloudera.org:8080/#/c/2627/1/init.sh File init.sh: Line 122: OS_NAME_VERSION=$(lsb_release -sir 2>&1) : # Remove spaces, trim minor versions, and convert to lowercase. : OS_NAME_VERSION=$(cut -d. -f1 <<< "$OS_NAME_VERSION" | tr "A-Z" "a-z") : case "$OS_NAME_VERSION" in : # "enterprise" is Oracle : centos* | enterprise* | redhat*) OS_NAME=rhel;; : debian*) OS_NAME=debian;; : suse*) OS_NAME=suse;; : ubuntu*) OS_NAME=ubuntu;; : *) echo "Warning: Unable to detect operating system" 1>&2 : OS_NAME=unknown;; : esac : OS_VERSION=$(echo "$OS_NAME_VERSION" | awk '{ print $NF }') I tried executing this on my Ubuntu 14 box and not sure if OS_VERSION is getting the right result. Did I maybe do something wrong? mj@mj-desktop:~/dev/native-toolchain$ echo $OS_NAME_VERSION ubuntu 14 mj@mj-desktop:~/dev/native-toolchain$ echo $OS_VERSION ubuntu 14 http://gerrit.cloudera.org:8080/#/c/2627/1/source/kudu/build.sh File source/kudu/build.sh: Line 23: URL PARCEL_REPO_URL? http://gerrit.cloudera.org:8080/#/c/2627/1/source/kudu/gen-stub-so.py File source/kudu/gen-stub-so.py: Line 38: sym_type maybe this needs to be upper cased, the nm docs show these as being either t/T or w/W respectively -- To view, visit http://gerrit.cloudera.org:8080/2627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie177b8eb3c36c04bd4e2d5257c68e87b000cae85 Gerrit-PatchSet: 1 Gerrit-Project: Toolchain Gerrit-Branch: master Gerrit-Owner: Casey Ching <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
