Code review comment for lp:~jelmer/bzr-dbus/zeitgeist

On Thu, 2010-02-18 at 02:54 +0000, Robert Collins wrote:
> On Thu, 2010-02-18 at 02:38 +0000, Jelmer Vernooij wrote:
> > > bzr-dbus is part of the canonical maintained plugins for bzr, so it needs
> > > copyright assignment.
> > >
> > > I don't see that you need the commit hook, as bzr-dbus is already connected to
> > > tip changes, which all commits are. I'd like you to have a look at that ;).
> > >
> > > Other than that conceptually sure its fine to have this live in -dbus.
> > I've updated the copyright - Markus told me he's already signed the contributor assignment.
> >
> > I think we should just support notifications of commits that were made by the local user for now and possibly supporti the notification of pull/push on branches later on
>
> I'd really rather not have duplicate code in the codebase, and it sure
> looks like that is what it is at the moment.
>
> Have you tried just logging what it grabs with the current hook - you
> might like it!
The 'duplication' is an extra hook, one line for the registration and
one line for the actual hook.

The alternative is filtering out commits in the current hook and only
calling the zeitgeist code in that case - that's still one or two lines
of code.

Using the post_branch_change_tip hook logs waaay too much data and
delutes gnome-activity-journal significantly at least for me, but I do a
lot of pulls.

Cheers,

Jelmer