Junio C Hamano <gitster@xxxxxxxxx> writes: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> cmn@xxxxxxxx (Carlos Martín Nieto) writes: >> >>> Junio C Hamano <gitster@xxxxxxxxx> writes: >>> >>>> Carlos Martín Nieto <cmn@xxxxxxxx> writes: >>>> >>>>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh >>>>> index e9019ac..93e5d6e 100755 >>>>> --- a/t/t3200-branch.sh >>>>> +++ b/t/t3200-branch.sh >>>>> @@ -383,6 +383,22 @@ test_expect_success 'use --set-upstream-to modify a particular branch' \ >>>>> test "$(git config branch.my13.remote)" = "." && >>>>> test "$(git config branch.my13.merge)" = "refs/heads/master"' >>>>> >>>>> +test_expect_success 'test --unset-upstream on HEAD' \ >>>>> + 'git branch my14 >>>>> + test_config branch.master.remote foo && >>>>> + test_config branch.master.merge foo && >>>>> + git branch --set-upstream-to my14 && >>>>> + git branch --unset-upstream && >>>>> + test_must_fail git config branch.master.remote && >>>>> + test_must_fail git config branch.master.merge' >>>>> + >>>>> +test_expect_success 'test --unset-upstream on a particular branch' \ >>>>> + 'git branch my15 >>>>> + git branch --set-upstream-to master my14 && >>>>> + git branch --unset-upstream my14 && >>>>> + test_must_fail git config branch.my14.remote && >>>>> + test_must_fail git config branch.my14.merge' >>>>> + >>>> >>>> What should happen when you say "--unset-upstream" on a branch B >>>> that does not have B@{upstream}? Should it fail? Should it be >>>> silently ignored? Is it undefined that we do not want to test? >>> >>> I'd say it should be ignored, as the end result we want is for there to >>> be no upstream information. What we do underneath is remove a couple of >>> config options, wich doesn't fail if they didn't insist in the first >>> place. >> >> I am not sure about that reasoning. >> >> $ git config --unset core.nosuchvariable ; echo $? >> 5 >> >> looks like a failure to me. >> >> More importantly, wouldn't we want to catch typo in >> >> git branch --unset-upstream mext >> >> which admittedly should come from a different codepath (I do not >> think your patch checks if "mext" branch exists before going ahead >> to the config--unset dance)? > > Sorry, the last paragraph was incomplete. > > In general, we should detect errors and allow the user to choose to > ignore. > > A script that wants to make sure that B does not have an upstream, > without knowing if it already has one, can say "--unset-upstream B" > and choose to ignore the error if B does not have an upstream. > > If the script is carefully written, it would try to check if B has > one and call "--unset-upstream B" only when it doesn't. A casually > written loose script would say "--unset-upstream B 2>/dev/null" > and keep going (it would not notice other kinds of errors, but that > is what makes it "casual and loose"). OK, that's a good point, I'll update the patch. cmn -- 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