Re: [PATCH] Documentation/fetch-options: emit recurse-submodules, jobs unconditionally

From: Stefan Beller <sbeller@google.com>
Date: Sep 26, 2016, 20:44 UTC
Message-ID: <CAGZ79kbeq+Wznm=ChWO0tU5A_haPQ6DvKNHbK+8Y3es5OVcRag@mail.gmail.com>
In-Reply-To: <xmqq7f9yqu9w.fsf@gitster.mtv.corp.google.com>

On Mon, Sep 26, 2016 at 1:29 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> Currently the section about recursing into submodules is repeated in
>> git-pull word for word as it is in fetch-options.
>>
>> Don't repeat ourselves here and include the --recurse-submodules via
>> fetch options.
>>
>> As a bonus expose the --jobs parameter in git-pull as well as that is
>> declared as a OPT_PASSTHRU for fetch internally already.
>
> The above may not be technically wrong, but smells like an
> under-researched description.

I knew this question was coming, but the research was not as easy to follow as it seemed convoluted to me.

After a bit more research, I think 8f0700dd33f (fetch/pull: Add the 'on-demand' value to the --recurse-submodules option) is the culprit, where this patch should have been squashed into, as that made the both locations word for word equal.

Show 6 quoted lines

>
> So where did we go wrong?  Was there a good reason why we have two
> instances of these option descriptions, and if so, are we sure that
> that reason is no longer applicable to today's system that we can
> safely share the description?  The proposed log message is a place
> to answer these questions.

The commit above is where we went wrong; It doesn't seem like a good reason for not including this is given in there.

Show 5 quoted lines

>
> By the way, 7dce19d3 is interesting in another way and worth
> studying in that it adds --submodule-prefix ;-) It may be something
> we want to consider consolidating with what Brandon has been working
> on.

That's why Brandon is cc'd now. :)

Show 9 quoted lines

>
> By the way^2, the "unconditionally" on the title conveyed less
> information than their bits weigh.  Unless a reader knows
> fetch-options are shared between fetch and pull, s/he would not know
> you meant by "unconditionally" to show these in both fetch and pull.
>
>     Documentation: share more descriptions for options between fetch and pull
>
> perhaps?

That's way better. I'll resend shortly.

Thanks, Stefan