All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: David Tran <unsignedzero@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3] tests: use "env" to run commands with temporary env-var settings
Date: Wed, 19 Mar 2014 12:55:06 -0700	[thread overview]
Message-ID: <xmqqfvme2cbp.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1395168845-1972-1-git-send-email-unsignedzero@gmail.com> (David Tran's message of "Tue, 18 Mar 2014 18:54:05 +0000")

David Tran <unsignedzero@gmail.com> writes:

> Originally, we would use "VAR=VAL command" to execute a test command with
> environment variable(s) only for that command. This does not work for commands
> that are shell functions (most notably test functions like "test_must_fail");
> the result of the assignment is retained and affects later commands.
>
> To avoid this, we assigned and exported the environment variables and run
> the test(s) in a subshell like this,
>
> 	(
> 		VAR=VAL &&
> 		export VAR
> 		test_must_fail git command to be tested
> 	)
>
> Using the "env" utility, we should be able to say
>
> 	test_must_fail git command to be tested
>
> which is much short and easier to read.

Looks familiar ;-) but it seems the changes from the original you
took it from all look worsening, not improvements, to me.

>>Isn't GIT_CONFIG here another way of saying:
>>
>>	test_must_fail git config -f doesnotexist --list
>>
>>Perhaps that is shorter and more readable still (and there are a few
>>similar cases in this patch.
> I'll ignore this for now. If needed I can make another patch to resolve this.

Yes, I think that is sensible.  And it does not have to be done by you.

> Hopefully this should be all of it.

Seems to be well done.  Thanks.

      parent reply	other threads:[~2014-03-19 19:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <244379@gmane.comp.version-control.git>
2014-03-18 18:54 ` [PATCH v3] tests: use "env" to run commands with temporary env-var settings David Tran
2014-03-19  3:59   ` Eric Sunshine
2014-03-19 19:55   ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfvme2cbp.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=unsignedzero@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.