From: Stefan Beller <sbeller@google.com>
Date: Tue, 12 Jan 2016 23:32:15 GMT
Subject: Re: [PATCH v3 3/3] submodule: extend die message on failed checkout with depth argument
Message-ID: <CAGZ79kY-tzhKxYeSe_cknBP0f8hPZJ9e1+AXq88XjJevc_Dteg@mail.gmail.com>
In-Reply-To: <BCA2C81C-8DB4-4695-8A44-79B51103A684@gmail.com>

On Tue, Jan 12, 2016 at 1:29 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>
> On 11 Jan 2016, at 17:26, Stefan Beller <sbeller@google.com> wrote:
>
>> On Sun, Dec 20, 2015 at 3:19 PM,  <larsxschneider@gmail.com> wrote:
>>
>>> +test_expect_success 'submodule update clone shallow submodule on non-default branch' '
>>> +       git clone cloned super4 &&
>>> +       pwd=$(pwd) &&
>>> +       (cd super4 &&
>>> +        sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
>>> +        mv -f .gitmodules.tmp .gitmodules &&
>>> +        test_must_fail git submodule update --init --depth=3 2>submodule.out &&
>>
>> Why do we choose a depth of 3 here?
> I just copied this code from the test above. Let's change that to "--depth=1" to avoid confusion.
> Should I reroll or can this be fixed in the merge?

I noticed that too after reviewing 1&2 that 3 seems to be the standard
depth in the tests,
so I'd think 3 is fine here, too. Which makes it not worth to reroll,
so I'd be fine with it as is.

>
>>
>>> +        test_i18ngrep --count "Commit is probably not on the default branch." submodule.out
> I was just reading this and wonder if we should remove "--count". My initial intention was to
> make sure this error message only happens once. However, this is not checked here anyways.
>
>>> +    )
>>> '
>>>
>>> test_expect_success 'submodule update --recursive drops module name before recursing' '
>
>
> Thanks,
> Lars