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

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
>>>>>> "jam" == John A Meinel <email address hidden> writes:
>
> >> There is little to comment on given the detailed cover letter, I
> >> like the cleanup (to come ? :) in annotate and the introduction
> >> of the Annotator class, but since you intend to build policy
> >> classes as front-end, why not make the class private until you
> >> feel more confident about the overall API ? Like you, I'm not
> >> sure the GUIs will really need to access that class...
>
> jam> I'm not sure if you understood me correctly. Annotator
> jam> is *meant* to be the final api for getting an annotation
> jam> for various versions of a file.
>
> Oh ! Indeed, I understood you wanted trees to be the primary interface.

So I'm trying to resolve the two issues. But a *tree* doesn't talk about
the history of a file. I might add:

 tree.get_annotator(file_id)

That would say something like: "give me an object that can talk about
the history of the file you have."

I need some way for the WT to inject the 'current' version, and have a
default to annotating that tip. I haven't worked out the details yet.

You *could* do this by having a bunch of revision trees for each
revision you want to annotate, and then having the annotator cached at
the VF layer. But it seems better to control the caching lifetime and
parameters in the GUI layer, rather than underneath trees inside VF.

...

>
> Then I'd go with:
>
> for head in heads: break # Get head from the set
>
> on a single line to make it more obvious.
>
> That's the first time I see that idiom, I will not be surprised
> next time (hopefully, but others can).
>
> <snip/>
>
> >> +=1 ? Hmm, I like that pyrex version you're using, send me some :)
>
> jam> Actually for pyrex 0.9.8 you can even do:
>
> Oh ! Yes, jaunty is still at 0.9.7.2 ...
>
> Reading the NEWS about it, I can only agree here.
>
> What is needed to have the package updated ? Host it in the bzr
> PPAs ?
>
> Vincent

I don't really know.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpUqcYACgkQJdeBCYSNAAN8nwCfXd9wjlU4U9L48k7ollH9qFqX
kzoAoJQPLsJPwIy7y2CcrjeyOrQFYYAI
=qWQI
-----END PGP SIGNATURE-----

« Back to merge proposal