Re: [PATCH 1/2] branch: allow creating a branch with same name and same starting point.

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2010, 19:38 UTC
Message-ID: <7vzl4cy11i.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1263737212-8101-2-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 8 quoted lines

> Previously, "git branch --track newname old" was rejected if newname
> existed, even if it already had the same value. This patch allows it,
> even without --force. This has two advantages:
>
> * Not requiring --force for a safe operation, hence allowing the user to
>   benefit from the other safety checks.
>
> * Allow changing the configuration of the checked-out branch.

Two issues I have to ask because you didn't cover these cases in your tests [*1*]:

 - What should "git branch new old" do when (0) local branch new already
   exists and points at old, (1) branch.new.{remote,rebase,merge} do not
   currently exist, and (2) branch.autosetup{merge,rebase} is set?

 - What should "git branch --no-track new old" do when (0) local branch
   new already exists and points at old, (1)
   branch.new.{remote,rebase,merge} do currently exist?

No matter what they do, should there be some extra message when the new code does something that the old code didn't do?

> +test_expect_success 'setup tracking with branch --track on existing branch' '
> ...
> +	git branch from-master master --track &&

The parser may be too loosely implemented and might allow this today, but please stick to "command names then dashed options and then arguments".

We have to fix the parser someday to be more strict and broken order in tests like this will get in the way.

> +	git config branch.from-master.merge > actual &&
> +	grep -q "^refs/heads/master$" actual
> +'
>  test_done

[Footnote]

*1* it is an easy trap to fall into to test only what it newly does while showing your shiny new toy and forget to test conditions that the shiny new toy should not kick in.