[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