Re: [PATCH v3] remote: add --fetch and --both options to set-url

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 25, 2014 at 6:48 AM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:
> git remote set-url knew about the '--push' option to update just the
> pushurl, but it does not have a similar option for "update fetch URL and
> leave whatever was in place for the push URL".
>
> This patch adds support for a '--fetch' option which implements that use
> case in a backwards compatible way: if no --both, --push or --fetch
> options are given, then the push URL is modified too if it was not set
> before. This is the case since the push URL is implicitly based on the
> fetch URL.
>
> A '--both' option is added to make the command independent of previous
> pushurl settings. For the --add and --delete set operations, it will
> always set the push and/ or the fetch URLs. For the primary mode of
> operation (without --add or --delete), it will drop pushurl as the
> implicit push URL is the (fetch) URL.

I've read (though perhaps not fully digested) the other email thread
which led up to this version of the patch, as well as the
documentation update in this patch, but I still don't understand the
need for the --both option. Intuitively, I would expect that
specifying --fetch and --push on the command line would have the same
effect as the proposed --both option. Thus, why is a separate (and
exclusive) --both option needed? Is it to reduce typing? What am I
missing?

More below.

> The documentation has also been updated and a missing '--push' option
> is added to the 'git remote -h' command.
>
> Tests are also added to verify the documented behavior.
>
> Signed-off-by: Peter Wu <peter@xxxxxxxxxxxxx>
> ---
>
>  v2: fixed test case
>  v3: added --both option, changed --fetch --push behavior, added more tests for
>      --add/--delete cases, refactored to reduce duplication (not special-casing
>      add_mode without oldurl, just skip initially setting oldurl).
>
> Translators note: the help text gained more translatable strings and some
> strings got additional options.
> ---
> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> index cb103c8..bdd0305 100644
> --- a/Documentation/git-remote.txt
> +++ b/Documentation/git-remote.txt
> @@ -134,7 +134,16 @@ Changes URL remote points to. Sets first URL remote points to matching
>  regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
>  <oldurl> doesn't match any URL, error occurs and nothing is changed.
>  +
> -With '--push', push URLs are manipulated instead of fetch URLs.
> +With '--both', both the fetch and push URLs are manipulated.
> ++
> +With '--fetch', only fetch URLs are manipulated.
> ++
> +With '--push', only push URLs are manipulated.
> ++
> +If none of the '--both', '--fetch' or --push' options are given, then
> +'--both' applies only if no push URL was set before. Otherwise '--fetch'
> +is assumed for historical reasons. This default may change in the
> +future to '--both' to avoid surprises depending on the configuration.

This explanation is somewhat tortuous. Assuming that the --both option
is superfluous, perhaps this could be explained more clearly along
these lines:

--- 8< ---
`--fetch` changes fetch URLs, and --push changes push URLs. Specified
together, both fetch and push URLs are changed.

For historical reasons, if neither --fetch nor --push is specified,
then the fetch URL is changed, as well as the push URL if not already
set.
--- 8< ---

>  +
>  With '--add', instead of changing some URL, new URL is added.
>  +
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]