On Thu, May 08, 2014 at 09:41:22PM +0200, Per Cederqvist wrote:
> 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 devil's in the details :)
> 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
Much better than the original, IMO.
Thanks,
Jeff.
--
We have joy, we have fun, we have Linux on a Sun...
--
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