[MERGE] annotation policies

[John Arbash Meinel]

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

John Arbash Meinel wrote:
| 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.
|

This is just an extension of the earlier patch, which now implements a couple
more types, which might show what I was thinking about earlier.

John
=:->


|
| 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

iEYEARECAAYFAkhQXZcACgkQJdeBCYSNAAOomQCeMdq5j29knz6QZ2q4XSM5MtxQ
PvQAniReus7JseUe+/5p2wk0DSVxlPzn
=TIWh
-----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/b26cb1fb/attachment-0001.diff