[RFC/MERGE] annotation policies

[John Arbash Meinel]

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

I've been working on allowing multiple annotation policies to be selected at run
time. Probably the hardest part for me is to get a decent way of testing all of
them. So I'd like some feedback about what I've done to date, before I go too
much father.


For the AnnotationPolicy code, I have a base (abstract) class in
bzrlib.annotion_policy.AnnotionPolicy. Which only really has 1 public member
(reannotate). This basically just replaces the bzrlib.annotate.reannotate function.

I have a basic registry of different policies, which are generally expected to
be subclasses, but only really have to have a constructor that takes a heads
provider, and has a reannotate() function.

The base AnnotationPolicy provides the bulk of the work for reannotating texts.
However, I have in mind a couple different policies that would override more of
the functionality.

I still have to do performance testing of it, but the basic idea is to make the
bulk of the matching super fast, and then the odd "ambiguous" line can be
matched more slowly.


For the testing strategy, the plan is to provide different AnnotationScenarios.
(bzrlib.tests.test_annotation_policy.AnnotationScenario is the base class.)

Each scenario applies to either all polices, or is customized to a specific
policy. I chose to use a mixed instantiation/inheritance to do the structuring.
The idea is that a class is set up with the basic texts, etc. And then you
instantiate that class with a given policy, and include information about how
that policy interprets the annotations differently.

I'd like some feedback about this structure. It is flexible enough for what I
want to do, and I feel comfortable adding more scenarios easily. But it might be
overly complex for someone coming along who wants to understand WTF is going on.
I need someone else to look at it to tell me so, though.

My biggest concern is the disconnect when someone overrides the annotation in an
instance. I didn't want to have to retype the whole text, so I have you supply
what lines get overridden. But honestly, I can realize that the actual texts are
going to be reasonably sized, so it could be a lot clearer if we just require to
override the whole text.

I added a few redundancy checks into the testing code, so that if you set up
your test incorrectly, it *should* error out on the test setup side, rather than
failing in odd ways during testing. (Making you think your policy is wrong, when
it was a bad setup.) However, that means that it will fail during the startup of
"bzr selftest" rather than while the tests are actually running. I might be able
to push down the actual compiling until later.

There is also a bit of a naming issue, because I call the class
AnnotationScenario, and TestScenarioAdapter has a different meaning for
"scenario". (one is a class, the other is a dictionary of member_variable=>value
pairs.)

Anyway, I'm pretty happy with where this is going, and I think I can make it
perform well and give the user some flexibility with how they want their
annotations. Arguably the policy could encompass even more, and have a way to
indicate what parent texts it is going to need. (I'm thinking of a left-only
annotation policy which should be super fast, and wouldn't need us to extract
all of those right-hand parents, and meets the needs of the python group of
telling you who allowed this code into mainline, rather that what random user
wrote it.)

I think this can grow into that, though. No reason to design it all at the
beginning.

(I'm only giving 'MERGE' because I want BB to track it so it doesn't get lost.
And I suppose it provides identical behavior at the moment, so it might be nice
to merge into bzr.dev so I can do incremental updates from here.)

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

iEYEARECAAYFAkhQI38ACgkQJdeBCYSNAAO/7wCcDtMRQr2ndbYOo0SD59WF2dXy
SEoAn0iUaqpmlL6BSOuCLg5LYLnO3QqV
=fYor
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rfc_annotation_policy.patch
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20080611/d3846cbf/attachment-0001.diff