SZEDER Gábor <szeder....@gmail.com> writes:

> On Mon, Oct 15, 2018 at 03:12:00AM -0700, Johannes Schindelin via 
> GitGitGadget wrote:
>> diff --git a/ci/lib.sh b/ci/lib.sh
>> index 06970f7213..8532555b4e 100755
>> --- a/ci/lib.sh
>> +++ b/ci/lib.sh
>> @@ -1,5 +1,26 @@
>>  # Library of functions shared by all CI scripts
>>  
>> +if test true = "$TRAVIS"
>> +then
>> +...
>> +    export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
>> +    export GIT_TEST_OPTS="--verbose-log -x --immediate"
>> +fi
>
> Please set all these variables ...

Do you mean "VAR=VAL; export VAR" is kosher, "export VAR=VAL" is
not?

>> @@ -81,7 +102,6 @@ check_unignored_build_artifacts ()
>>  # and installing dependencies.
>>  set -ex
>
> ... after we turn on 'set -x', so the variables' values will be
> visible in the logs.

Ah, no, you didn't.  Although I think both are valid points, I think
ci/lib.sh is expected to be used only inside a more predictable
environment (e.g. we know the shell used is not a random POSIX shell
but one that is happy with "export VAR=VAL"), so it should be OK.
Showing the values of these variables in the log may still be good
idea.

> (Or move this 'set -ex' to the beginning of the script?  Then we
> could perhaps avoid similar issues in the future.)

Sure (provided that it is an issue to begin with---if we are
interested in the value of TRAVIS_BRANCH, for example, being able to
see it only because "CI_BRANCH=$TRAVIS_BRANCH" assignment is made
feels a bit sideways---we'd be better off explicitly logging
anything we are interested in in the longer term, no?).

Reply via email to