[MERGE] Multiple element keys.
Robert Collins
robertc at robertcollins.net
Sat Jul 28 02:10:31 BST 2007
On Fri, 2007-07-27 at 13:15 -0400, John Arbash Meinel wrote:
>
> v- This code path could use some comments
> + if len(elements):
>
> v- You know how many depths you need to go, by how many elements have
> not been
> consumed (how many are None).
> I think it would be better to make use of that, then to use
> isinstance/type ==
> dict.
Well I thought about that, and this is within the opaque data type - it
doesn't leak to be confusing to other developers, and for this really we
should profile different approaches and find the fastest. That said I
don't expect this disk layout to last, as per my other emails. So I'm
inclined to do the simplest thing for now.
>
> + # XXX: To much duplication with the GraphIndex class;
> consider
> finding
> + # a good place to pull out the actual common logic.
>
> ^- I was about to point it out, before I saw this. Isn't this a
> straight
> copy
> of the other code?
Not quite. Theres some superficial differences that are easily corrected
by changing object member names. More deeply, one is to become page
based, *possibly* keeping a parsed version, and one remains memory
based. So I am thinking it needs a mixin or other form of composition to
make it clean. Until then I want the definately in-memory one clearly
separated from the 'read from disk' index.
>
> + def _keys_to_version_ids(self, keys):
> + return tuple(key[0] for key in keys)
>
> ^- Why is this returning a tuple rather than a list?
>
> return [key[0] for key in keys]
>
> Seems a much more natural fit.
It was a lot more natural to be a tuple before I refactored the keys to
be tuples :).
-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: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070728/e114f639/attachment.pgp
More information about the bazaar
mailing list