Code review comment for lp:~lifeless/bzr/apply-inventory-delta

Robert Collins wrote:
> On Thu, 2009-07-16 at 03:28 +0000, Ian Clatworthy wrote:
>
>
>> Doc changes and tests all look fine. More code feedback: I don't think
>> it's safe removing these few lines from inventory.py ...
>>
>
> Why not?
>
>
>> for old_path, file_id in sorted(((op, f) for op, np, f, e in
>> delta
>> if op is not None),
>> reverse=True):
>> - if file_id not in self:
>> - # adds come later
>> - continue
>> # Preserve unaltered children of file_id for later
>> reinsertion.
>> file_id_children = getattr(self[file_id], 'children', {})
>>
>
> If file_id is not in self, then the old_path must be None, or the delta
> is deleting a file_id that isn't present. Thats a corrupt delta and we
> should not silently accomodate that.
>
> -Rob
>
>
You need to trap the KeyError self[file_id] can throw if you remove it.

Ian C.