[MERGE] add a hook for Branch.set_last_revision_info()
Andrew Bennetts
andrew at canonical.com
Tue Apr 8 14:58:18 BST 2008
James Henstridge wrote:
> On 08/04/2008, Andrew Bennetts <andrew at canonical.com> wrote:
> > James Henstridge wrote:
> > > One feature of the bzr-dbus plugin is to send a D-Bus signal over the
> > > session bus whenever the head revision of a branch changes. It
> > > currently does this through the "set_rh" hook.
> >
> >
> > bb:comment
> >
> > I haven't reviewed the code in detail yet, but this seems to have a lot of
> > overlap with https://lists.ubuntu.com/archives/bazaar/2008q2/040066.html
>
> Yes it does. I didn't see that branch on bundlebuggy and missed it on
> this list. Ian's interface would cover my use cases, so I'd be happy
> with either API.
>
> It does look like Ian's patch is missing tests though.
I do prefer Ian's API. As a general rule, passing an object rather than a bunch
of simple arguments to a plugin is a much more flexible API:
- it's easy to extend the interface of the object without breaking existing
plugins.
- not all plugins that use a hook will necessarily need all the information.
An object can provide methods to retrieve information which might be
expensive to calculate, so plugins don't pay the price for details they don't
use.
Also, specifically for this hook, I think it is important that the previous tip
revision info is accessible to the hook. That will make it possible to enforce
a particular policy (e.g. all .xml files must be well formed, or no tabs in .py,
etc) for all revisions on a branch, even when a push adds multiple revisions in
a single operation. Similarly it makes it possible to generate sensible
commit-like emails when there are multiple commits — or an uncommit.
That said, tests are important! Any chance you could make a patch that combines
the best of both worlds? :)
-Andrew.
More information about the bazaar
mailing list