Merge lp:~jelmer/zeitgeist-datasources/bzr-plugin-improvements into lp:zeitgeist-datasources/0.8

Dude that is awesome
Thanks alot

On Tue, Nov 29, 2011 at 12:18 PM, Jelmer Vernooij <email address hidden> wrote:

> Jelmer Vernooij has proposed merging
> lp:~jelmer/zeitgeist-datasources/bzr-plugin-improvements into
> lp:zeitgeist-datasources.
>
> Requested reviews:
> Zeitgeist Data-Sources Team (zeitgeist-dataproviders)
>
> For more details, see:
>
> https://code.launchpad.net/~jelmer/zeitgeist-datasources/bzr-plugin-improvements/+merge/83758
>
> Various improvements to the bzr plugin:
>
> * use standard functionality for pretty formatting URLs: prevents
> stripping off first 7 characters off non-local URLs
> * track push events
> * don't include location information in events - that data is already
> present in the subject
> * factor out branch
> --
>
> https://code.launchpad.net/~jelmer/zeitgeist-datasources/bzr-plugin-improvements/+merge/83758
> Your team Zeitgeist Data-Sources Team is requested to review the proposed
> merge of lp:~jelmer/zeitgeist-datasources/bzr-plugin-improvements into
> lp:zeitgeist-datasources.
>
> === modified file 'bzr/__init__.py'
> --- bzr/__init__.py 2011-08-31 13:13:33 +0000
> +++ bzr/__init__.py 2011-11-29 11:17:28 +0000
> @@ -30,10 +30,10 @@
> Copy this directory to ~/.bazaar/plugins/zeitgeist/*
> """
>
> -import time
> from bzrlib import (
> branch,
> trace,
> + urlutils,
> )
>
> _client = None
> @@ -62,35 +62,45 @@
> return _client
>
>
> +def subject_for_branch(branch):
> + from zeitgeist.datamodel import Subject, Interpretation, Manifestation
> + location = urlutils.unescape_for_display(branch.base,
> "utf-8").decode("utf-8")
> + return Subject.new_for_values(
> + uri=unicode(branch.base),
> + interpretation=unicode(Interpretation.FOLDER),
> + manifestation=unicode(Manifestation.FILE_DATA_OBJECT),
> + text=u"Bazaar branch at %s" % location,
> + origin=unicode(branch.base),
> + mimetype="application/x-bzr",
> + )
> +
> +
> +def get_actor():
> + # FIXME: Allow overriding this by e.g. qbzr and bzr-gtk
> + return u"application://bzr.desktop"
> +
> +
> def post_commit(local, master, old_revno, old_revid, new_revno,
> new_revid):
> client = get_client()
> if client is None:
> return
> - from zeitgeist.datamodel import Event, Subject, Interpretation,
> Manifestation
> + from zeitgeist.datamodel import Event, Interpretation, Manifestation
> revision = master.repository.get_revision(new_revid)
> if new_revno == 1:
> interpretation = Interpretation.CREATE_EVENT
> else:
> interpretation = Interpretation.MODIFY_EVENT
> - _text = _("Commited on: ")
> - _text += master.base[7:-1] #don't considere file://
> - _text += _(" Revision no. : ")
> + _text = _(" Revision no. : ")
> _text += str(new_revno) + "\n"
> _text += revision.message.rstrip()
>
> - subject = Subject.new_for_values(
> - uri=unicode(master.base),
> - interpretation=unicode(Interpretation.FOLDER),
> - manifestation=unicode(Manifestation.FILE_DATA_OBJECT),
> - text=unicode(_text),
> - origin=unicode(master.base),
> - )
> + subject = subject_for_branch(...

I had to do some more changes

1) branch.Branch.hooks.install_lazy_named_hook does not exist.
Changed it to branch.Branch.hooks.install_lazy_named_hook

2) hooks.py was not added to Makefile.am

Rest everything is fine

Another thing I noticed is that bzr.desktop does not exist. I could only find
bzr-explorer.desktop
bzr-handle-patch.desktop
bzr-notify.desktop

$ dpkg -S /usr/share/applications/bzr-*
bzr-explorer: /usr/share/applications/bzr-explorer.desktop
bzr-gtk: /usr/share/applications/bzr-handle-patch.desktop
bzr-gtk: /usr/share/applications/bzr-notify.desktop

So shall the datasource install the .desktop file?

I have made some changed which includes the ones I mentioned above and also changed post_pull to use WORLD_ACTIVITY instead of USER_ACTIVITY and bzr.desktop to bzr-notify.desktop

Should we add bzr.desktop file with the datasource of prefer to use the ones already present?

My branch with changes are here
lp:~manishsinha/zeitgeist-datasources/bzr-plugin-improvements-manish

Hi Manish,

Thanks for those fixes. I think they're all good with the exception of the change from bzr.desktop to bzr-notify.desktop. bzr-notify is a tool that just displays notification windows, it's not an appropriate tool for opening branches.

I would prefer this datasource to ship with a desktop file which opens nautilus as Exec and path as the argument. Jelmer, your suggestion?

Using nautilus seems reasonable to me too.

> I would prefer this datasource to ship with a desktop file which opens
> nautilus as Exec and path as the argument. Jelmer, your suggestion?

What if nautilus is not installed?

> > I would prefer this datasource to ship with a desktop file which opens
> > nautilus as Exec and path as the argument. Jelmer, your suggestion?
> What if nautilus is not installed?
Isn't that an issue with all of the actors? bzr-notify also isn't necessarily installed.

> What if nautilus is not installed?

We can't fix all the situations.

Other option would be to make nautilus a hard dependency of this datasource. I don't think that would be good.

> > > I would prefer this datasource to ship with a desktop file which opens
> > > nautilus as Exec and path as the argument. Jelmer, your suggestion?
> > What if nautilus is not installed?
> Isn't that an issue with all of the actors? bzr-notify also isn't necessarily
> installed.

My point was that such resolution doesn't actually solve anything, and arguably shipping an "invalid" desktop file is worse than having an actor that's not a real desktop file.

Right, ideally we wouldn't be setting an actor at all because the actor is unknown. It's something that just happens to be using bzrlib.

I made some changes which are in
lp:~manishsinha/zeitgeist-datasources/bzr-plugin-improvements-manish

In this, I have created a bzr-logger.desktop file along with bzr-logger-icon-64.png
The actor is set as bzr-logger.desktop and the desktop file contains

Name=Bazaar Logger
Comment=Logs bazaar activities
Exec=nautilus %U
Icon=bzr-logger-icon-64.png

This looks like the best compromise from all ends. It shows up as Bazaar icon and when you click on it (say from GAJ) it will open that folder using nautilus which contains the files for that branch

Right now our bazaar datasource logs are not being used apart from GAJ (which is broken half of the time). I have a plan in mind to use the logs to create a dashboard kind of application. That was the reason I needed the revision number to be stored somewhere (now it is stored in text filed) and also the mimetype and actor fields.

Creating our own desktop and pixmap file is the only way to fill the actor field without introducing a new dependency on a bigger (not installed) application like bzr-gtk or bzr-explorer.

On Wed, Mar 21, 2012 at 07:55:20PM -0000, Manish Sinha (मनीष सिन्हा) wrote:
> I made some changes which are in
> lp:~manishsinha/zeitgeist-datasources/bzr-plugin-improvements-manish
>
> In this, I have created a bzr-logger.desktop file along with bzr-logger-icon-64.png
> The actor is set as bzr-logger.desktop and the desktop file contains
>
> Name=Bazaar Logger
> Comment=Logs bazaar activities
> Exec=nautilus %U
> Icon=bzr-logger-icon-64.png
>
> This looks like the best compromise from all ends. It shows up as Bazaar icon and when you click on it (say from GAJ) it will open that folder using nautilus which contains the files for that branch
This seems a bit odd - nautilus doesn't actually display or record bzr activities.

Isn't actor supposed to be the application that actually was
responsible for the event, rather than some application for viewing it?

Would it be an option to just not set an actor at all?

Cheers,

Jelmer

It can be a way to not set the actor at all, but how will be find out that it is bazaar event?

My desktop thing was a workaround to make the events more usable for applications esp GAJ. Without that GAJ and other applications have to do more work on their side to display events

OK so here is my issue. It looks great however please fix subject_for_branch

You should return a list of subjects
the first subject being the folder and then another subject for each of the files that were edited or created

Am 21/03/12 23:40, schrieb Seif Lotfy:
> Review: Needs Fixing
>
> OK so here is my issue. It looks great however please fix subject_for_branch
>
> You should return a list of subjects
> the first subject being the folder and then another subject for each of the files that were edited or created
For the commit hook, I think that the only subject really is the branch
(not even the working tree). The files that are committed aren't being
changed themselves. Or am I misunderstanding the data model?

For push/pull/merge/update operations we are changing the working tree
so it does indeed make sense to take those items as subjects too.

Cheers,

Jelmer

I updated the code as needed and posted it here

https://code.launchpad.net/~manishsinha/zeitgeist-datasources/bzr-improvements-subjects/+merge/99168