Hi Antony,
Antony T Curtis wrote:
> On Fri, 2007-06-22 at 17:01 +0200, Ingo Strüwing wrote:
>> antony@stripped wrote:
>> ..
>>> ChangeSet@stripped, 2007-06-19 23:50:17-07:00, antony@stripped +7 -0
>>> Bug#25513
>>> "Federared Transactions Failure"
>> ...
>>> This patch builds on Bug29019 and Bug25511
>> Sigh. So I must review these first.
>>
>
> Alas, all three bugs step on the same area: Inserts on Federated.
>
> Although, I have tried to keep each cset as self contained as possible
> such that seperate review should be possible.
Sure. I appreciate that. However, you have built one patch on top of the
other. So when I compare the result of a later patch with the unpatched
5.0 tree, I see changes that are not subject of that patch. So I would
have to agree with all changes at once. If I have issues with changes
that belong to a former patch, I'd better comment on that patch than on
the latest one.
When reviewing in the order of the patches, then I see familiar changes
in later patches. That's much easier for me. And I won't need to comment
on changes that I've commented in a former review already.
If you need to change a former patch, the later patches won't work
anymore and need changes too.
I think it would have been easier to base all patches on the unpatched
5.0 tree. Intersections between the patches would then be handled during
merging, where we do not do reviews.
But please don't change the patches any more as I am already working on
their reviews and don't want to repeat that effort unnecessarily.
Regards
Ingo
--
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Radlkoferstr. 2, D-81373 München
Geschäftsführer: Kaj Arnö - HRB München 162140
