[MERGE] Use slots consistently in InventoryEntry hierarchy -- update

[Robert Collins]

On Wed, 2006-06-07 at 14:43 +0100, Martin Pool wrote:
> On  1 Jun 2006, Jan Hudec <bulb at ucw.cz> wrote:
> 
> > Ok. I have tried to update the patch to address this and Aaron's concerns.
> > The 'kind' is now a normal class attribute and so are all the other
> > attributes specific to some InventoryEntry subclass. So the serializers can
> > safely read all of them and get None for the unapplicable ones, so they are
> > kind-agnostic again (I actually reverted them to the old code except
> > replacing the 'something != None' constructs with proer 'something is not
> > None'.
> > 
> > Please review. The original version got +1, -0, +1. I am attaching both the
> > complete diff from mainline (ok, 3 revisions back at this point) and a diff
> > of just the updates. Please use
> > http://drak.ucw.cz/~bulb/bzr/bzr/inventory-slots for merging.
> 
> +1, ok with me.  Thanks.

This has two current problems:
 * Tests fail - showstopper.
 * The class attributes on InventoryEntry are IMNSHO quite ugly.

I'd really love to see:
InventoryEntry has slots for name, parent_id, file_id
InventoryFile has those slots and slots for text_sha and size
InventoryLink has the I.E. slots and symlink_target
InventoryDirectory has children.

There should be no attribute on class or instance for e.g.
symlink_target except on InventoryLink.

This *may* increase the amount of current 'if kind ==' tests in the
codebase, but I think that is an acceptable *interim* tradeoff. Each one
of those should have a #FIXME attached to it to use an appropriate means
of instance based dispatch - either a method on InventoryEntry, or
double-dispatch via a visit() style method.

I realise this patch is getting ridiculous in terms of thrashing, so I
ask that Martin/John/Aaron agree on this goal I'm setting so we dont
waste Jan's effort.

-1 (sorry) as it stands.
Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060608/c9ba5e75/attachment.pgp