Re: [PATCH 1/4 v4] submodules: make submodule-prefix option

From: Brandon Williams <bmwill@google.com>
Date: Sep 27, 2016, 20:29 UTC
Message-ID: <20160927202925.GA32565@google.com>
In-Reply-To: <xmqqtwd1nr56.fsf@gitster.mtv.corp.google.com>

On 09/27, Junio C Hamano wrote:

Show 26 quoted lines

> Brandon Williams <bmwill@google.com> writes:
> 
> > +--submodule-prefix=<path>::
> > +	Set a prefix which gives submodules context about the superproject that
> > +	invoked it.  Only allowed for commands which support submodules.
> 
> This, and also the message in die(), uses a phrase "support
> submodules", but it is unclear what it exactly means to the end
> users and readers.
> 
> A "ls-files" that is recursively run as an implementation detail of
> the "grep --recurse-submodules" would be taught to support this
> option with this series.  Who is supporting submodules in that
> context?
> 
> I'd imagine (close to) 100% of the people would say it is "grep"
> that is supporting submodules, not "ls-files", but what this
> paragraph and die() message want to express by the phrase "support
> submodules" is the fact that "ls-files" knows how to react to
> "--submodule-prefix" option.
> 
> I'd suggest not to worry too much about this phrasing at this point,
> until we figure out exactly how we want to present these to end
> users.  For now, perhaps drop the second sentence and replace it
> with "The end-users are not expected to use this option" or
> something like that?

K can do. The intention is that each command has to do whatever internal rework needed so that it understands how to interact with subomodules (be that calling another command or internally supporting it).

> > +		die("%s doesn't support submodules", p->cmd);
> 
> s/submodules/submodule-prefix/ at least.

So should the #define be SUPPORT_SUBMODULE_PREFIX instead? That may be too narrow minded and not looking toward future submodule options support but I'm not sure.

-- 
Brandon Williams