Code review comment for lp:~jameinel/bzr/1.17-rework-annotate

Revision history for this message
Vincent Ladeuil (vila) wrote :

>>>>> "jam" == John A Meinel <email address hidden> writes:

    jam> ...
    >>
    >> A couple of comments on the class API:
    >>
    >> - _update_needed_children() and _get_needed_keys() sounds like
    >> good candidates for Graph() or some specialization of it.

    jam> True, though they also have side effects like removing
    jam> texts when there are no more needed children, etc.

I see.

<snip/>

    >> - add_special_text(), hmm, what's that ? The doc string doesn't
    >> help a lot :-) Does that need to be public ?

    jam> It does, as it is used by WorkingTree to add the 'current:' text to be
    jam> annotated.

Haaa ! Worth mentioning then :)

    jam> (One other benefit of this new code is that 'bzr
    jam> annotate NEWS' after a merge doesn't annotate both
    jam> parents independently... \o/)

Hurrah !

       Vincent

« Back to merge proposal