[PATCH] Changesets for bzr

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:

...

>>> Well, I can run 'bzr send-changeset' and have it fail. So somehow we are.
> 
> ~/bzr/w-changeset$ ./bzr send-changeset
> bzr: ERROR: unknown command 'send-changeset'
> 
> That's the kind of failure I get.

I just didn't have the latest version. 10 revisions ago it was still
doing 'register_command(cmd_send_changeset)' I pulled your latest, and
it is correct now.

...

>>> I don't really prefer an api where you always have to call a second
>>> function if you call the first. It just feels wrong. If one requires the
>>> other to be called, then it should call it.
> 
> If you want to get technical, node_add, note_file_id, and usually
> note_patch or note_target are all required in order to have a valid
> ChangesetTree.
> 
> The design was a precursor to TreeTransform, and like TreeTransform
> gives you the freedom to make the changes in any order.  A future format
> might note the add long after the execute bit status was known.  Or vice
> versa.
> 
> But also like TreeTransform, I'm happy to build convenience methods on
> top of the basic API.

Okay. I do think it is generally better to only call one thing, so that
you don't forget anything. But I can see your point that we might have
the patches up front, and then metadata at the bottom, and one not know
about the other yet.

> 
>>>>>> I thought with the inventory fixes we might either have no executable
>>>>>> attribute, or it would at least be None for non-files.
>>>> Your ChangesetTree patch also sets an execute bit value for non-files.
>>>>
>>>> But this is actually correct.  The execute bit is a member of
>>>> InventoryEntry, so it applies to symlinks and directories.  It is set
>>>> False by default.  So now I'm asserting that all inventory entries have
>>>> an execute bit, and it's False for non-files.
> 
>>> I don't know if you followed it, but there was work done on making
>>> InventoryEntry uses __slots__ everywhere, rather than just on the base
>>> class. And at that time they removed a bunch of the members that don't
>>> make sense to all classes.
> 
> But just as you said "if a file doesn't exist, then it's not
> executable", one could say "if it's not a file, then it's not executable".
> 
> Anyhow, w-changeset is compatible with the current semantics.  If they
> change, we can update it.
> 
>>> But we can clean it up when that gets merged. (I've +1'd it, but waiting
>>> on a second review).
> 
> I don't understand slots, so I haven't reviewed it.
> 
> Aaron

Slots just limit what member variables you can use. And then instead of
using a __dict__, they use indexed lookups. So doing:

class Foo(object):
  __slots__ = ['a', 'b']

  def __init__(self):
    self.a = 1
  def func1(self):
    self.b = 2
  def func2(self):
    self.c = 3

calling func2 will fail, because Foo cannot have a member named 'c'. If
you comment out the __slots__ line, then it would succeed.

There seems to be a large advantage for data classes to use slots. They
 use less memory, and seem to have faster access times.
http://www.python.org/doc/2.3.5/ref/slots.html

There are some caveats:
http://mail.python.org/pipermail/python-dev/2003-August/037542.html

Martin did some performance testing on Weave and inventory which showed
that __slots__ was actually very useful. We just haven't been faithful
to keep slots active.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060526/6938e72f/attachment.pgp