[MERGE] Implement xml8 serializer

Aaron Bentley aaron at aaronbentley.com
Wed Apr 2 14:52:30 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> Aaron Bentley wrote:
>> Hi all,
>>
>> This patch implements the xml8 serializer which will be used by the
>> upcoming pack-1.4 format.

> This needs a NEW entry.

It doesn't seem worthy of a NEWS entry.  The pack-1.4 format will merit
a NEWS entry, but until it lands, this isn't used for anything.

> Also, xml8._check_revisions() needs its
> docstring updated: "By default no checking is done." is no longer correct.

Thanks.

> I also like how you replaced some of the asserts with explicit checks, e.g.
> 
>> -        assert elt.tag == 'inventory'
>> +        if elt.tag != 'inventory':
>> +            raise errors.UnexpectedInventoryFormat('Root tag is %r' % elt.tag)
> 
> It would be nice to do this in xml5._unpack_inventory() as well.

What happened here was I moved the xml5 version into xml5.py, replacing
it with the xml6 version.

>> This patch changes the inheritance so that older formats inherit from
>> new ones.  This means that the current format is all in one place, and
>> we can drop support for old formats easily.  It also makes it easy for
>> future formats to use the same numbers for their revision and inventory
>> formats.
> 
> I'm good with this approach. The one thing that stands out though as a
> consequence is that a bunch of "general" escaping functions have moved
> from xml5.yyy to xml8.yyy. If and when we were to drop support for xml8
> in N years time, we'd arguably be caught doing the same shuffle again.

No, I'm proposing a new regime where older formats inherit from newer
ones.  So when we introduce xml9, we'll have to change the names again.

> That's a long way of saying that they arguably have nothing to do with
> any particular version and ought to be moved to a more generic module,
> e.g. xml_serialiser or xmlescape, say. I'm not saying that's required to
> land this, though it does seem as good a time as any to put them in the
> "right" place given we're moving them as part of this patch.

Well, either that or make them static methods.

> Finally, I did a code review of xml_serializer in order to gain enough
> context to review this. This bit is either wrong or confusing:

Looks wrong to me.  Possibly it's unused.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFH84+e0F+nu1YWqI0RAstSAJ0V7dhsrw3H6c5keXWnpZ11cAOCbACfXDNq
cg1Vc8YGP6mVdKoyFSX0NKY=
=QBEy
-----END PGP SIGNATURE-----



More information about the bazaar mailing list