From: Elijah Newren <newren@gmail.com>
Date: Mon, 09 Aug 2010 19:22:40 GMT
Subject: Re: [PATCHv3 1/2] t5520-pull: Add testcases showing spurious conflicts from git pull --rebase
Message-ID: <AANLkTin0xvK8H8RvaXtmaNHJh1dStn0AN1Mqed7JGvuO@mail.gmail.com>
In-Reply-To: <7vfwyn61td.fsf@alter.siamese.dyndns.org>

On Mon, Aug 9, 2010 at 1:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Elijah Newren <newren@gmail.com> writes:
<snip>
>> +modify () {
>> +     sed -e "$1" < "$2" > "$2".x &&
>> +     mv "$2".x "$2"
>> +}
>
> Just a style thing but I'd prefer to see the above written like this:
>
>        modify () {
>                sed -e "$1" <"$2" >"$2.x" &&
>                mv "$2.x" "$2"
>        }

I copied this function verbatim from t/t4127-apply-same-fn.sh.  Would
you like me to fix that one too?

>> +test_expect_success 'setup for avoiding reapplying old patches' '
>> +     (cd dst &&
>> +      git rebase --abort;
>
> This may be hypothetical but this discards error condition from failing to
> ch into dst (for whatever reason).  Don't we expect "git rebase --abort"
> to exit with a non-zero status?  Same comment for the last one in the
> patch below.

Yes, Hannes pointed out the same issue.  Does the follow-up interdiff
I posted in response to my patch address this in a way you'd like?