[brisbane-core/MERGE] inventory fix & refactor

ian.clatworthy at internode.on.net ian.clatworthy at internode.on.net
Wed Mar 4 23:22:53 GMT 2009


On Thu 05/03/09 12:57 AM , Vincent Ladeuil v.ladeuil+lp at free.fr sent:

> Are you trying to make CHKInventory mutable ?

Well, I *was* heading that way but I won't now. I'm OK  with
CHKInventory having a different API to Inventory but we'll
need to make that very clear for bzrlib users (like plugins)
that xxx.inventory no longer has a pile of methods that it
used to.

> Ian> Altogether, there are currently 15 public methods on Inventory
> Ian> that CHKInventory doesn't support.

> All of them requiring a mutable inventory I presume ?

No, just 6 of the 15. I can't see any reason for not supporting
the (other 9) read-oriented APIs - it just isn't done yet.

> I'm not sure trying to make CHKInventory mutable is the way to
> go[1], if you need a mutable inventory, just use Inventory,
> CHKInventory._get_mutable_inventory was added for cases like
> that.

Well, there's lots of problems with this new method: it doesn't
work, it's private, it's poorly documented and it's unclear how or
why it's different to copy().

> [1] Your patch already makes 28 tests error out (after an
> obvious rename of delta into inventory_delta in and one fail because
> you introduced many TABs)

Sorry about that. My environment is my Mum's laptop for a few
days and it's only partially setup for development. Having said that,
I'm really sure I ran 'bzr selftest inventory' before commiting/sending
and it mostly worked (bar some DirState locking issues which I
understood to be a known Windows issue).

Ian C.





More information about the bazaar mailing list