Re: [PATCH v3 4/4] submodule_needs_pushing() NEEDSWORK when we can not answer this question

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2016, 00:13 UTC
Message-ID: <xmqqtwb8s2m8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kYyyjP7W7gWq6WomVSkhRtMbZZMKYQPFszko4_f9oprgg@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 10 quoted lines

>> "We do not know" ...
>
> ... because there is no way to check for us as we don't have the
> submodule commits.
>
>     " We do consider it safe as no one in their sane mind would
>     have changed the submodule pointers without having the
>     submodule around. If a user did however change the submodules
>     without having the submodule commits around, this indicates an
>     expert who knows what they were doing."

I didn't think it through myself to arrive at such a conclusion, but to me the above sounds like a sensible reasoning [*1*].

Show 11 quoted lines

>>   We currently
>> +                * proceed pushing here as if the submodules commits are
>> +                * available on a remote. Since we can not check the
>> +                * remote availability for this submodule we should
>> +                * consider changing this behavior to: Stop here and
>> +                * tell the user how to skip this check if wanted.
>> +                */
>>                 return 0;
>
> Thanks for adding the NEEDSWORK, I just wrote the above lines
> to clarify my thought process, not as a suggestion for change.

One thing I would suggest would be "Stop here, explain the situation and then tell the user how to skip". I am not convinced that it would _help_ users and make it _safer_ if we stopped here, though.

> Overall the series looks good to me; the nits are minor IMHO.

Ditto.

[Footnote]

*1* My version was more like "we do not know if they would get into
    a situation where they do not have enough submodule commits if
    we pushed our superproject, but more importantly, we DO KNOW
    that it would not help an iota if we pushed our submodule to
    them, so there is no point stopping the push of superproject
    saying 'no, no, no, you must push the submodule first'".