git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Wu <peter@lekensteyn.nl>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Git List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3] remote: add --fetch and --both options to set-url
Date: Tue, 25 Nov 2014 23:53:56 +0100	[thread overview]
Message-ID: <2086201.pfQdnB4m0z@al> (raw)
In-Reply-To: <CAPig+cS1rFpFO4YsNEi2Fjzfj+qH7Oat+PR+0GUo32=MFTNRiA@mail.gmail.com>

On Tuesday 25 November 2014 17:09:04 Eric Sunshine wrote:
> On Tue, Nov 25, 2014 at 6:48 AM, Peter Wu <peter@lekensteyn.nl> 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?

The initial version just added --fetch and let --fetch --push behave
like the current --both. Here you can see the most recent discussion
leading to the --both option:
http://www.spinics.net/lists/git/msg242336.html

There was an overlapping discussion about the confusing historic
behavior (default vs. --fetch vs. --push --fetch), and an alternative
(less verbose form) of --push --fetch. The reason that --both is
introduced probably has something to do with it being less verbose.
I am not too attached to either option by the way.

> > 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@lekensteyn.nl>
> > ---
> >
> >  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< ---

Excellent, short and concise. If this path is taken, I would still add
the note about the future change.

I am also interested in opinions about the suggested default behavior.
Should it become --both (--push --fetch)? In that case '--both' will be
a pretty useless option in the future (when it becomes the default).

> >  +
> >  With '--add', instead of changing some URL, new URL is added.
> >  +

  reply	other threads:[~2014-11-25 22:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-25 11:48 [PATCH v3] remote: add --fetch and --both options to set-url Peter Wu
2014-11-25 22:09 ` Eric Sunshine
2014-11-25 22:53   ` Peter Wu [this message]
2014-12-04  9:37 ` Jeff King
2014-12-17 10:08 ` Torsten Bögershausen
2014-12-17 14:20   ` Peter Wu
2014-12-17 14:28     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2086201.pfQdnB4m0z@al \
    --to=peter@lekensteyn.nl \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).