[PATCH 1 of 2 RFC V2] largefiles: changed overridelog to work with graphlog

[Mads Kiilerich]

On 03/06/2014 07:54 PM, Lucas Moscovicz wrote:
> On 3/6/14, 10:26 AM, "Mads Kiilerich" <mads at kiilerich.com> wrote:
>> Did it already not work and this is a bugfix? Or is it a change of
>> strategy that will make it work with the refactoring in the next
>> changeset?
> This was not working already for graph log. In fact, if you try most of
> the tests in test-largefiles.t with -G option you get a different output.
> This is supposed to fix those tests and leave everything working for the
> next patch where I change the log code to also use the graph log option
> parser.

Ok. It would be nice to have that spelled out in the commit message.

>>
>>> Tests for graphlog to be added in future patches.
>> Why not introduce the tests in a patch before this or in this patch so
>> we can see that it improves (or at least doesn't break)?
> Tests for graph log will break before this patch. I am not sure whether I
> am supposed to add them and send this patches together but as I understand
> the flow for this should include this patch first and the tests later. Is
> there another way to get around this?

If it is broken as in 'gives slightly incorrect result' then I would 
prefer to first have a patch that introduce test coverage of this area 
and documents how it is. Obviously, that test should pass. Next, this 
patch would update the test result and it would be very clear what this 
patch would change.

AFAIK Matt doesn't like that approach very much so usually bugfixes 
(like this one) also introduces a test that makes sure we don't get 
regressions later on but doesn't show what changed.

/Mads