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] IMPALA-5905: add script for all-build-options job
Date Tue, 12 Sep 2017 22:39:18 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5905: add script for all-build-options job
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8043/1/bin/all-build-options.sh
File bin/all-build-options.sh:

Line 1: #!/usr/bin/env bash
> Nit: I don't like the file name. all-build-options.sh sounds like it's some
Yeah the jenkins/ subdirectory makes sense to me, there's a lot of junk in bin/


PS1, Line 35:     do
            :       OPTIONS[4]=$BUILD_SHARED_LIBS
            :       if ! ./bin/clean.sh
            :       then
            :         echo "Clean failed"
            :         exit 1
            :       fi
> You could remove noclean in line 26 and remove this. It's obviously not exa
I just preserved this logic from the original Jenkins job script. I didn't add this so I don't
know if there was some deeper motivation or it.

I could probably try to clean this and other things up but I'd prefer to check this in with
the minimal changes required to make it work then worry about cleanup later.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6e78f05c41e3ccd59af599b00e453e7f88b2bb34
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message