Re: [RFC PATCH 2/2] push: support pushing to a remote group

[Junio C Hamano <gitster@pobox.com>]

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

> -	remote = pushremote_get(repo);
> -	if (!remote) {
> -		if (repo)
> -			die(_("bad repository '%s'"), repo);
> -		die(_("No configured push destination.\n"
> -		    "Either specify the URL from the command-line or configure a remote repository using\n"
> -		    "\n"
> -		    "    git remote add <name> <url>\n"
> -		    "\n"
> -		    "and then push using the remote name\n"
> -		    "\n"
> -		    "    git push <name>\n"));
> +	if (repo) {
> +		if (!add_remote_or_group(repo, &remote_group))
> +			die(_("no such remote or remote group: %s"), repo);
> +	} else {
> +		remote = pushremote_get(NULL);
> +		if (!remote)
> +			die(_("No configured push destination.\n"
> +			    "Either specify the URL from the command-line or configure a remote repository using\n"
> +			    "\n"
> +			    "    git remote add <name> <url>\n"
> +			    "\n"
> +			    "and then push using the remote name\n"
> +			    "\n"
> +			    "    git push <name>\n"));
>  	}

The basic idea to use "remote" (the default remote cannot be multiple)
vs "remote_group" (the command line gave which remotes to talk with)
sounds good.

But I started wondering what happens when the command line gave a
single remote to talk with.  Probably we want a code that does

	if (remote_group has only one remote)
		remote = take the sole remote from the remote_group;

here before we continue.  Or the other way around and we handle the
"default remote cannot be multiple" case as a special case, e.g.

	if (remote) {
		create remote_group with a single member "remote";
		remote = NULL;
	}

and then we do not have to do ...

> +	/*
> +	 * set_refspecs and mirror detection must not use `remote`
> +	 * when it may be NULL (group path). For the single-remote case,
> +	 * handle them here. For the group case they are handled
> +	 * per-remote inside the loop below.
> +	 */

... "handle them here because single-remote is special" at all, no?

I would prefer to avoid "X must be done for each remote in the
remote-group, but Y can be done only once", as future developers
will get it wrong when they add their own Z and consider which side
Z falls into.  The code structure that removes special case would
help by making sure that a singleton case is special only because
the loop over remote_group runs once, and otherwise there is nothing
special goes on.

Thanks.