From: Francesco Pretto <ceztko@gmail.com>
Date: Mon, 06 Jan 2014 17:52:43 GMT
Subject: Re: [PATCH 1/2] git-submodule.sh: Support 'checkout' as a valid update command
Message-ID: <CALas-iiQO8OwhS_W9u3sNDYuWf_3XnFsF11NZGLnnt_+pDTtVA@mail.gmail.com>
In-Reply-To: <xmqq8uutj9c9.fsf@gitster.dls.corp.google.com>

Ok, applying the suggested modifications and resending shortly.

Thank you,
Francesco

2014/1/6 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "W. Trevor King" <wking@tremily.us> writes:
>>
>>> On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
>>>> +                   case "$update_module" in
>>>> +                   '')
>>>> +                           ;; # Unset update mode
>>>> +                   checkout | rebase | merge | none)
>>>> +                           ;; # Known update modes
>>>> +                   !*)
>>>> +                           ;; # Custom update command
>>>> +                   *)
>>>> +                           update_module=
>>>> +                           echo >&2 "warning: invalid update mode for submodule '$name'"
>>>> +                           ;;
>>>> +                   esac
>>>
>>> I'd prefer `die "…"` to `echo >&2 "…"`.  It's hard to know if mapping
>>> the user's preferred (unknown) update mechanism to 'checkout' is
>>> serious or not.
>>>
>>> This commit also makes me think that --rebase, --merge, and --checkout
>>> should be replaced with a single --update={rebase|merge|checkout|!…}
>>> option, but that's probably food for another commit (and a long
>>> finger-breaking deprecation period).
>>
>> All of the above points sound sensible to me.
>
> I'll tentatively queue this on 'pu' (with the suggested "die"
> update), with some rewording of the log message.  The patch needs to
> be signed-off, though.
>
> Thanks.