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

184 + def _entries_to_incorporate(self):

^- I wonder if this could be written without tree.inventory.

All the path2id calls can just be replaced with tree.path2id(). The call for "if root_id in inventory" can be replaced with "tree.id2path(file_id)" (we may need to trap an exception, though, which is a bit ugly.)

This:
  subdir = other_inv[subdir_id]

Gets replaced by:
  subdir = other_tree.iter_entries_by_dir(subdir_id).next()[1]

Which isn't very pretty either, but should do the job.

We don't have to, but it might be nice to get away from accessing the .inventory attribute.

_Wrapper is ugly, but I can understand why you do it. I'm a bit curious why we don't need to have __setattr__ defined, though.

I haven't gone through all the tests yet, but I figured I'd give some feedback and start the conversation.