Re: [PATCH 2/3] branch: add --unset-upstream option

From: Carlos Martín Nieto <cmn@elego.de>
Date: Aug 27, 2012, 17:30 UTC
Message-ID: <87a9xgnsxx.fsf@centaur.cmartin.tk>
In-Reply-To: <7vpq6h2tmk.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 30 quoted lines

> Carlos Martín Nieto <cmn@elego.de> 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.

Would you like a test that makes sure two --unset-upstream in a row don't cause any errors?

cmn