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

Revision history for this message
John A Meinel (jameinel) wrote :

Hash: SHA1

On 10/10/2012 9:23 PM, Zygmunt Krynicki wrote:
> Review: Needs Information
>> 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

They can implement _find_ancestors() and they should be queried if

_find_ancestors was meant as a way to dig more quickly into ancestors
when the data storage isn't great at random lookups (rather than
proceeding step-by-step into ancestors, grab whatever ancestors you
can get cheaply, and return).

Anyway, the implementation for BTreeBuilder can be trivial (return 1
generation of ancestors). It is a performance optimization to have a
tighter inner loop. If you don't, it will just call the function again.


Version: GnuPG v1.4.12 (Cygwin)
Comment: Using GnuPG with Mozilla -


« Back to merge proposal