Code review comment for lp:~zyga/bzr/find_ancestors

> I did see that branch but I recall reading that the solution is incorrect. I
> am unable to judge that myself. Could you please suggest if skipping
> BTreeBuilder instances makes sense? If not, should the (can they) implement
> the same _find_ancestors() method?

The bug is not terribly clear, but as I read it:

* The first branch just copied the method over, which is not correct
* No one commented on Wouter's branch, which implements _find_ancestors differently

It's not clear to me if it really makes sense for the builder to have a _find_ancestors method, but without a test that demonstrates the situation in which one is present in the graph, it's hard to understand.

> Also, if you could point me out to any docs that describe how bzr internals
> work would help me a lot.

Refer to the docs/developers directory, also available online:

<http://doc.bazaar.canonical.com/bzr.dev/developers/>

See particularly:

<http://doc.bazaar.canonical.com/bzr.dev/developers/overview.html>
<http://doc.bazaar.canonical.com/bzr.dev/developers/indices.html>
<http://people.canonical.com/~mwh/bzrlibapi/bzrlib.btree_index.html>

Also the relevant parts of:

<http://doc.bazaar.canonical.com/bzr.dev/developers/HACKING.html>
<http://doc.bazaar.canonical.com/bzr.dev/developers/testing.html>