From: Eric Sunshine <sunshine@sunshineco.com>
Date: Tue, 23 Jun 2015 19:17:08 GMT
Subject: Re: [PATCHv6 1/3] git-rebase -i: add command "drop" to remove a commit
Message-ID: <CAPig+cRbtNV8YfvOQNPD_wEiyEkyDeTtbahYA-Vd9Q+9uv6z9w@mail.gmail.com>
In-Reply-To: <1637608183.731706.1435086079947.JavaMail.zimbra@ensimag.grenoble-inp.fr>

On Tue, Jun 23, 2015 at 3:01 PM, Remi Galan Alfonso
<remi.galan-alfonso@ensimag.grenoble-inp.fr> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > +test_rebase_end () {
>> > +       test_when_finished "git checkout master &&
>> > +       git branch -D $1 &&
>> > +       test_might_fail git rebase --abort" &&
>> > +       git checkout -b $1 master
>> > +}
>>
>> The way this is indented makes it difficult to see that lines 2 and 3
>> are continuations of 1. Perhaps format it like this instead?
>>
>>     test_rebase_end () {
>>         test_when_finished "git checkout master &&
>>             git branch -D $1 &&
>>             test_might_fail git rebase --abort" &&
>>         git checkout -b $1 master
>>     }
>
> I completely agree with you, moreover it was indented like this before.
> I'll change it in my local version for now.
>
> Ironically, it was modified after the following:
>
> Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > > +test_expect_success 'rebase -i respects rebase.missingCommitsCheck=ignore' '
>> > > +       test_config rebase.missingCommitsCheck ignore &&
>> > > +       test_when_finished "git checkout master &&
>> > > +               git branch -D tmp2" &&
>> >
>> > Strange indentation.

Clearly, that guy who made the "Strange indentation" review comment
didn't know what he was talking about. ;-)

In that earlier review, I must have missed the fact that the quoted
string spanned multiple lines, and, unfortunately, got too busy with
other things to say "ah, the indentation makes perfect sense" when
your response pointed out its true nature.

Matthieu's suggestion of formatting it like:

    test_when_finished "
        ... &&
        ...
    " &&

should help to avoid such misconceptions.