[MERGE] journalled inventory serialiser
Ian Clatworthy
ian.clatworthy at internode.on.net
Wed Mar 26 08:11:17 GMT 2008
Robert Collins wrote:
> The goal of this inventory serialiser work I'm doing is to:
> - move the delta logic for inventories from texts to inventory entries.
> - enable direct emitting of commit deltas avoiding large text diffs
> during commit
> - be a stepping stone in the path of improving the general class of
> 'operations between historical versions'
All important goals. I've been meaning to review this for quite some
time but (1) my other priorities and (2) its size have kept me from it.
Thanks for working on this - my profiling of bzr-fastimport showed over
and over again what a bottleneck our inventory handling is for
loading/committing revisions. Of the numerous approaches we've discussed
over time for inventories, I think this particular approach shows the
most promise.
FWIW, I grabbed your latest branch and reviewed that after merging with
bzr.dev. The development format stuff is now in bzr.dev so that reduced
the size of the change down to something more reasonable. Could you
consider breaking the patch into at least two parts to further
simplifying reviewing please? The first part can be just the
inventory.txt changes; the second part could be the rest.
bb:resubmit
Some initial feedback/questions ...
inventory.txt:
* The outcomes from John's review comments and your reply need to
be incorporated
* Why did you choose 'version:' as opposed 'revision:'?
(My first reaction was that the version was already included
in the "format:" field.)
* Grammar/spelling tweaks:
* s/a inventory/an inventory/
* s/hueristic/heuristic/
* s/lexographically/lexigraphically/ ?
inventory.py:
* make_inv_delta() ought to have explicit tests given its public
journalled_inventory.py:
* executable bit is not being serialised/deserialised
(at least not in the way that inventory.txt says it is)
* _reference_content() ought to be renamed to _tree_content()
for consistency with _tree_to_entry()
* s/it's parent/its parent/
* s/its done/it's done/
* no need to set last_modified to None on line 180.
No harm though.
A start,
Ian C.
More information about the bazaar
mailing list