Re: [GUILT 07/28] Added test cases for "guilt fold".

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 7, 2014 at 11:06 PM, Jeff Sipek <jeffpc@xxxxxxxxxxxxxx> wrote:
> On Wed, May 07, 2014 at 10:59:56PM +0200, Per Cederqvist wrote:
>> On Tue, May 6, 2014 at 9:40 PM, Jeff Sipek <jeffpc@xxxxxxxxxxxxxx> wrote:
>> > On Fri, Mar 21, 2014 at 08:31:45AM +0100, Per Cederqvist wrote:
>> >> Test that we can combine any combination of patches with empty and
>> >> non-empty messages, both with and without guilt.diffstat.  (All
>> >> patches are empty.)
>> >
>> > I feel like we should have *some* content there - most of the time, I care
>> > more about the diffs getting folded than the commit message :)
>>
>> I added these tests for a reason: the reproduce a bug in guilt that I found.
>>
>> I'm afraid that having some content might hide the bug I found. (I'm also
>> equally afraid that it might uncover other bugs in guilt, which would delay
>> integration of this patch series. So adding more test cases with content
>> is a good thing to do, but maybe not in this patch series.)
>
> Fair enough.  I use guilt-fold all the time and it hasn't lost any of my
> diffs, so I'm happy to defer this until some point in the future.
>
> ...
>> > for using_diffstat in true false ; do
>> >         for patcha in empty nonempty ; do
>> >                 for patchb in empty nonempty ; do
>> >                         echo "%% $patcha + $patchb (diffstat=$using_diffstat)"
>> >                         ${patcha}_patch $patcha
>> >                         ${patchb}_patch $patchb
>> >                         cmd guilt pop $patchb
>> >                         cmd guilt fold $patchb
>> >                         fixup_time_info $patcha
>> >                         cmd list_files
>> >                         cleanup $patcha
>> >                         cmd list_files
>> >                 done
>> >         done
>> > done
>> >
>> > Aha!  That's better, IMO.
>>
>> I'll try that and post a version 2 of the series. It might take a few
>> days, though.
>
> No problem.  I'm still the slower one of the two of us. :/
>
> Jeff.

There were a few details that made it a bit more complex than that,
but I think the end result was still an improvement.

The most obvious detail is that if you add two empty patches, you
cannot name them both "empty", so when $patcha and $patchb is the same
you have to add suffixes.

The other detail is that my tests used different commit messages when
both commits contained a message.  I want to retain that behaviour, so
that added a few lines of complexity.

I'll post an updated patch series once I've gone through all your
comments.  In the meantime, you can see my new implementation here:

http://repo.or.cz/w/guilt/ceder.git/commitdiff/3107dc73eaff020da18024c3b5f5f92b94d17852?hp=6df110c95133d6e557ce3dbcb6fd39bc797f877b#patch2

    /ceder
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]