Re: [PATCH 4/6] submodule--helper update-clone: allow multiple references

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 5, 2016, 21:06 UTC
Message-ID: <xmqq60rfvsbq.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kY4YxftihTP_cNqLrVTn0wrfNd5_mb40AB-t2beyzvdLA@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 7 quoted lines

>> -               ${reference:+--reference "$reference"} \
>> +               ${reference:+"$reference"} \
>
> Note how this changed the API of the submodule--helper.
> Currently we pass in --reference $reference
> and $reference consists of the string "--reference" and the actual
> reference. So it looked like '--reference' '--reference=foo'

So this change is a strict bugfix. The code without this patch had cmd_update that places "--reference=foo" in $reference, but the call to "git submodule--helper" it makes added an unnecessary "--reference" in front of it.

I was wondering why there is no corresponding change to add "--reference" on the code that calls cmd_update(). Thanks for saving my time ;-)

Perhaps that needs to go into the log message, though. Allowing multiple references is not that interesting from the POV of the codebase immediately after this step and only deserves a "by the way" mention.

    Subject: submodule--helper: fix cmd_update()

    cmd_update places "--reference=foo" in $reference, but the call to
    "git submodule--helper" it makes adds an unnecessary "--reference"
    in front of it.  The underlying "git clone" was called with two
    command options "--reference" and "--reference=foo" because of
    this.

    While at it, prepare the code to accept multiple references, as
    the underlying "git clone" can happily take more than one.

or something like that.

Needless to say, "While at it" could become a separate step, and a pure bugfix part may even want to become a separate and more urgent topic that can go to 'maint', with a test to prevent regression.