Code review comment for lp:~spiv/bzr/merge-into-merger

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

Robert Collins wrote:
> Review: Needs Fixing
> id2path('random') -> None
> So I think that that is fine.

You're thinking 'path2id':
>>> t.id2path('xxx')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 4, in id2path_read_locked
  File "bzrlib\workingtree_4.py", line 449, in id2path
    raise errors.NoSuchId(tree=self, file_id=file_id)
bzrlib.errors.NoSuchId: The file id "xxx" is not present in the tree
<WorkingTree6 of C:/Users/jameinel/dev/bzr/bzr.dev>.

>>> print t.path2id('xxx')
None

(Yes, they aren't symmetric.)
>
> I'm still ambivalent about avoiding inventory - inventory is the /model/ for the tree, after all. Anyhow, looks doable here.

Well, mostly it is about not having to compute 30k InventoryEntries when
you only want 1. Potentially we could make a much lazier inventory, but
we certainly aren't there (today) for Dirstate code.

>
> _finish_transform doesn't pair well with compute_transform. Perhaps finish_computing_transform?
>
> The new exception should be in errors.py until we change our policy on that.
>
> Rather than _Wrapper I suggest MergeTypeParameteriser. Or something like that. Long but less 'zomg I need to read the code' than Wrapper ;)
John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwwmT4ACgkQJdeBCYSNAAODlgCfTdNGyxYp7ts8f5M4E83GgRKc
O5wAoJh18G4KfahG62bKlmuf987Cyztx
=sCtl
-----END PGP SIGNATURE-----