Re: [PATCH] for-each-repo: do nothing on empty config

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2021, 20:50 UTC
Message-ID: <xmqqturtrdre.fsf@gitster.c.googlers.com>
In-Reply-To: <CAPig+cRGWzz5n_PZ=_OiHy2hkmeru3=fo2zX3_hnsEhnPq+giQ@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 9 quoted lines

> Is the behavior implemented by this patch desirable? That is, if the
> user mistypes the name of the configuration variable, would it be
> better to let the user know about the problem by returning an error
> code rather than succeeding silently? Or do you foresee people
> intentionally running the command against an empty config variable?
> That might be legitimate in automation situations. If legitimate to
> have an empty config, then would it be helpful to somehow distinguish
> between an empty config variable and a non-existent one (assuming we
> can even do that)?

I am guessing, without reading the mind of those who designed the feature, that the envisioned use pattern is that a configuration variable is designated, "git for-each-ref --config=<var> <cmd>" is carved into stone in a script that is run periodically with the hardcoded variable name in it, but the value of the variable may change from time to time. If it is expected that the variable can sometimes be empty, then erroring out or even issuing a warning would be counter-productive.

Show 6 quoted lines

> Is the API of this command ideal? It feels odd to force the user to
> specify required input via a command-line option rather than just as a
> positional argument. In other words, since the config variable name is
> mandatory, an alternative invocation format would be:
>
>     git for-each-repo <config-var> <cmd>

Or not to use any configuration variable (or not to have the for-each-repo subcommand at all) and let the users write something along the lines of...

	git config --get-all <config-var> |
	while read repo
	do
		( cd "$repo" && <cmd> )
	done

which is not all that bad and much more flexible.

> Or do you foresee future enhancements expanding the ways in which the
> repo list can be specified (such as from a file or stdin or directly
> via the command-line), in which case the `--config` option makes
> sense.