From: Johannes Sixt <j6t@kdbg.org>
To: Mehul Jain <mehul.jain2029@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Eric Sunshine <sunshine@sunshineco.com>,
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 6/7] t5520: reduce commom lines of code
Date: Sun, 3 Apr 2016 08:46:42 +0200 [thread overview]
Message-ID: <5700BC52.1040404@kdbg.org> (raw)
In-Reply-To: <CA+DCAeT4rF2dqL9iU9WYQJuxiSYstY9AaT2Hc7OvhmFNyKEgAg@mail.gmail.com>
Am 03.04.2016 um 08:24 schrieb Mehul Jain:
> On Sun, Apr 3, 2016 at 12:20 AM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 02.04.2016 um 19:58 schrieb Mehul Jain:
>>> +for i in --autostash --no-autostash
>>> +do
>>> + test_expect_success "pull $i (without --rebase) is illegal" '
>>> + test_must_fail git pull $i . copy 2>err &&
>>> + test_i18ngrep "only valid with --rebase" err
>>> + '
>>> +done
>>
>>
>> Hm. If the implementation of test_expect_success uses the variable, too, its
>> value is lost when the test snippet runs. Fortunately, it does not.
>>
>> You can make this code a bit more robust by using double-quotes around the
>> test code so that $i is expanded before test_expect_success is evaluated.
>
> I think that the current format is preferred over the one you suggest.
> Here[1] Junio
> has given a descriptive explanation.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/283350/focus=284769
Junio has a point there, of course.
In this light, I suggest that you use a more verbose variable name.
-- Hannes
next prev parent reply other threads:[~2016-04-03 6:50 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-02 17:58 [PATCH v2 0/7] t5520: tests for --[no-]autostash option Mehul Jain
2016-04-02 17:58 ` [PATCH v2 1/7] t5520: use consistent capitalization in test titles Mehul Jain
2016-04-02 17:58 ` [PATCH v2 2/7] t5520: ensure consistent test conditions Mehul Jain
2016-04-02 17:58 ` [PATCH v2 3/7] t5520: use better test to check stderr output Mehul Jain
2016-04-02 17:58 ` [PATCH v2 4/7] t5520: factor out common code Mehul Jain
2016-04-03 20:03 ` Eric Sunshine
2016-04-02 17:58 ` [PATCH v2 5/7] " Mehul Jain
2016-04-03 20:05 ` Eric Sunshine
2016-04-02 17:58 ` [PATCH v2 6/7] t5520: reduce commom lines of code Mehul Jain
2016-04-02 18:50 ` Johannes Sixt
2016-04-03 6:24 ` Mehul Jain
2016-04-03 6:46 ` Johannes Sixt [this message]
2016-04-02 17:58 ` [PATCH v2 7/7] t5520: test --[no-]autostash with pull.rebase=true Mehul Jain
2016-04-03 20:11 ` Eric Sunshine
2016-04-03 8:47 ` [PATCH v2 6/7] t5520: reduce commom lines of code Mehul Jain
2016-04-03 20:17 ` [PATCH v2 0/7] t5520: tests for --[no-]autostash option Eric Sunshine
2016-04-04 7:31 ` Matthieu Moy
2016-04-04 16:58 ` Mehul Jain
2016-04-04 17:00 ` Junio C Hamano
2016-04-04 17:07 ` Mehul Jain
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=5700BC52.1040404@kdbg.org \
--to=j6t@kdbg.org \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mehul.jain2029@gmail.com \
--cc=sunshine@sunshineco.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.