Code review comment for lp://staging/~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:

This bug can be triggered in a very simple way using bzr-fastimport. If this helps I can prepare reproducible instructions. This prevents one from using bzr-fastimport to consume a patch.

> * 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.

I read that as a request for reproducible instructions. I'm on it!

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

Thanks a lot, I'll get right to it!