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

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

> Thanks for having a go at fixing this.

Thanks for the feedback!

> To land, this branch really needs:
> * Tests, to demonstrate the problem and evaluate any attempted fix against
> * Logic, rather than than just raising NotImplementedError and skipping that
> case

Sure, at the moment it's just an early thing I managed to do (having no understanding of bzr internals) that allowed me to use bzr.

> Did you have a look at Wouter's branch, which implements the method? Does that
> also work for you?

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?

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

Thanks
ZK

review: Needs Information

« Back to merge proposal