Re: [PATCH v2] format-patch: introduce format.outputDirectory configuration

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 22, 2015, 00:05 UTC
Message-ID: <CAPig+cQsfTz7UzVcB1D8iu5on0o7E9nOv=WtzrR-GBmm=cdEpw@mail.gmail.com>
In-Reply-To: <xmqqk2rj8kf5.fsf@gitster.mtv.corp.google.com>

On Mon, Sep 21, 2015 at 6:41 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 28 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>> On Fri, Jun 19, 2015 at 2:28 PM, Alexander Kuleshov
>> <kuleshovmail@gmail.com> wrote:
>>> We can pass -o/--output-directory to the format-patch command to
>>> store patches not in the working directory. This patch introduces
>>> format.outputDirectory configuration option for same purpose.
>>>
>>> The case of usage of this configuration option can be convinience
>>
>> s/convinience/convenience/
>>
>>> to not pass everytime -o/--output-directory if an user has pattern
>>
>> s/everytime/every time/
>>
>>> to store all patches in the /patches directory for example.
>>>
>>> The format.outputDirectory has lower priority than command line
>>> option, so if user will set format.outputDirectory and pass the
>>> command line option, a result will be stored in a directory that
>>> passed to command line option.
>>>
>>> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
>>> ---
>
> Looks like there were mostly editorial niggles and no fundamental
> flaws in the design of the patch; it is somewhat a shame to make all
> the efforts go to waste.  Will we be seeing an update soon?

Looking at the new test a bit more closely...

    test_expect_success "format-patch format.outputDirectory option" '
        git config format.outputDirectory "patches/" &&
        git format-patch master..side &&
        cnt=$(ls | wc -l) &&
        test $cnt = 3 &&
        test_config format.outputDirectory "patches/" &&
        git config --unset format.outputDirectory
    '

I'm wondering what it's really testing. I presume that it wanted to count the number of files in the 'patches/' directory, however, the 'ls' is being invoked in the test trash directory instead. It turns out that the trash directory has three entries at this point, so the test succeeds, but entirely by accident.