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

Revision history for this message
Martin Packman (gz) wrote :

> 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:


See particularly:


Also the relevant parts of:


« Back to merge proposal