Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

On Tue, May 10, 2016 at 5:11 PM, Junio C Hamano wrote: > SZEDER Gábor writes: >> I wonder if is it really necessary to specify the path to the .git >> directory via $GIT_DIR. Would 'git --git-dir=/over/there' be just as >> good? > > Then you are testing two

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

SZEDER Gábor writes: > I wonder if is it really necessary to specify the path to the .git > directory via $GIT_DIR. Would 'git --git-dir=/over/there' be just as > good? Then you are testing two different things that may go through different codepaths. Adding yet another

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

Quoting Eric Sunshine : On Tue, May 10, 2016 at 3:12 PM, Eric Sunshine wrote: On Tue, May 10, 2016 at 2:39 PM, Jeff King wrote: On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote: while : do

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

On Tue, May 10, 2016 at 03:59:42PM -0400, Eric Sunshine wrote: > On Tue, May 10, 2016 at 3:48 PM, Eric Sunshine > wrote: > > Actually, I think we can have improved encapsulation and maintain > > readability like this: > > > > case "$1" in > > ... > > -g)

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

Eric Sunshine writes: > On Tue, May 10, 2016 at 3:12 PM, Eric Sunshine > wrote: >> On Tue, May 10, 2016 at 2:39 PM, Jeff King wrote: >>> On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote: while :

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

On Tue, May 10, 2016 at 3:48 PM, Eric Sunshine wrote: > Actually, I think we can have improved encapsulation and maintain > readability like this: > > case "$1" in > ... > -g) env="$2"; shift; shift ;; > ... > esac > > ... >

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

On Tue, May 10, 2016 at 3:12 PM, Eric Sunshine wrote: > On Tue, May 10, 2016 at 2:39 PM, Jeff King wrote: >> On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote: >>> while : >>> do >>> case "$1" in >>> -C)

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

Eric Sunshine writes: >> I don't know if it's worth worrying about or not. The usual solution is >> something like: >> >> env_git_dir=$2 >> env='GIT_DIR=$env_git_dir; export GIT_DIR' >> ... >> eval "$env" > > Makes sense; I wasn't quite happy with having $2

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

On Tue, May 10, 2016 at 2:39 PM, Jeff King wrote: > On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote: >> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh >> @@ -7,11 +7,13 @@ test_description='test git rev-parse' >> while : >> do >>

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote: > diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh > index c058aa4..525e6d3 100755 > --- a/t/t1500-rev-parse.sh > +++ b/t/t1500-rev-parse.sh > @@ -7,11 +7,13 @@ test_description='test git rev-parse' > test_rev_parse () { >