All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH 4/4] remote: use remote_is_configured() for add and rename
Date: Wed, 17 Feb 2016 15:24:55 +0100	[thread overview]
Message-ID: <20160217142455.GC1831@hank> (raw)
In-Reply-To: <alpine.DEB.2.20.1602171451030.6516@virtualbox>

On 02/17, Johannes Schindelin wrote:
> Hi Peff & Thomas,
>
> On Mon, 15 Feb 2016, Jeff King wrote:
> > This original is quite confusing. I thought at first that there was
> > perhaps something going on with allowing repeated re-configuration of
> > the same remote, as long as some parameters matched. I.e., I am
> > wondering if there is a case here that does _not_ segfault, that we
> > would be breaking.
> >
> > But reading over fb86e32dcc, I think I have convinced myself that it was
> > merely an ad-hoc check for "is_configured", and using that function is a
> > better replacement.
>
> Yes, yes, yes. Y'all are absolutely correct. I shoulda added a test case
> right away, to make sure not only that what I fixed does not get broken in
> the future, but also to document *what* was fixed, exactly.

Thanks for confirming and the test case so we don't break it again.

> So, belatedly, here goes a patch that verifies what that commit was
> supposed to fix, and yes, it passes with Thomas' changes (Junio, would you
> please apply this on top of tg/git-remote?):
>
> -- snipsnap --
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Wed, 17 Feb 2016 14:45:59 +0100
> Subject: [PATCH] t5505: 'remote add x y' should work when url.y.insteadOf = x
>
> This is the test missing from fb86e32 (git remote: allow adding
> remotes agreeing with url.<...>.insteadOf, 2014-12-23): we should
> allow adding a remote with the URL when it agrees with the
> url.<...>.insteadOf setting.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/t5505-remote.sh | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 94079a0..19e8e34 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -51,6 +51,11 @@ test_expect_success setup '
>  	git clone one test
>  '
>
> +test_expect_success 'add remote whose URL agrees with url.<...>.insteadOf' '
> +	git config url.git@host.com:team/repo.git.insteadOf myremote &&

Minor nit: I think we should use test_config here.

> +	git remote add myremote git@host.com:team/repo.git
> +'
> +
>  test_expect_success C_LOCALE_OUTPUT 'remote information for the origin' '
>  	(
>  		cd test &&
> --
> 2.7.1.windows.2
--
Thomas

  reply	other threads:[~2016-02-17 14:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-15 17:42 [PATCH 0/4] git remote improvements Thomas Gummerer
2016-02-15 17:42 ` [PATCH 1/4] remote: use skip_prefix Thomas Gummerer
2016-02-15 18:18   ` Jeff King
2016-02-15 18:35     ` Eric Sunshine
2016-02-15 18:36       ` Jeff King
2016-02-15 20:37     ` Thomas Gummerer
2016-02-15 17:42 ` [PATCH 2/4] remote: simplify remote_is_configured() Thomas Gummerer
2016-02-15 18:21   ` Jeff King
2016-02-15 20:38     ` Thomas Gummerer
2016-02-15 21:37       ` Junio C Hamano
2016-02-15 17:42 ` [PATCH 3/4] remote: actually check if remote exits Thomas Gummerer
2016-02-15 18:23   ` Jeff King
2016-02-15 17:42 ` [PATCH 4/4] remote: use remote_is_configured() for add and rename Thomas Gummerer
2016-02-15 18:33   ` Jeff King
2016-02-15 20:43     ` Thomas Gummerer
2016-02-17 13:54     ` Johannes Schindelin
2016-02-17 14:24       ` Thomas Gummerer [this message]
2016-02-17 16:20         ` Johannes Schindelin

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=20160217142455.GC1831@hank \
    --to=t.gummerer@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.