On Thu, Mar 3, 2016 at 11:01 PM, Matthieu Moy
<matthieu....@grenoble-inp.fr> wrote:
> There's no need to split code/test/doc into separate patches, except if
> your patches are getting really huge. As a reviewer, I like having tests
> and doc in the same patch because they describe the intention of the
> programmer.
>
> We try to have a history where each commit is equally good, and with
> your version there are two commits where --autostash exists and is
> undocumented (which is not catastrophic, though).

Alright, I will make a single patch for the next version.

>> --- a/t/t5521-pull-options.sh
>> +++ b/t/t5521-pull-options.sh
>> @@ -62,6 +62,22 @@ test_expect_success 'git pull -v --rebase' '
>>       test_must_be_empty out)
>>  '
>>
>> +test_expect_success 'git pull --rebase --autostash' '
>> +     mkdir clonedrbas &&
>> +     (cd clonedrbas  && git init &&
>> +     git pull --rebase --autostash "../parent" >out 2>err &&
>> +     test -s err &&
>> +     test_must_be_empty out)
>> +'
>> +
>> +test_expect_success 'git pull --rebase --no-autostash' '
>> +     mkdir clonedrbnas &&
>> +     (cd clonedrbnas  && git init &&
>> +     git pull --rebase --no-autostash "../parent" >out 2>err &&
>> +     test -s err &&
>> +     test_must_be_empty out)
>> +'
>
> Not sure these tests are needed if you have the ones in t/t5520-pull.sh.
> More tests means more time to run so testing twice the same thing has a
> cost.

I agree with you. This test may not be needed as t/t5520-pull.sh already test
this option.

Thanks,
Mehul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to