Moved runner "Amarok" to kdereview

Jan Gerrit Marker jangmarker at googlemail.com
Fri Oct 16 21:06:04 CEST 2009


Hello,
> * you need to send an announcement to kde-core-devel at kde.org as well
Done that. Marco Martin forwarded my mail about the "Kill" runner to it, so 
I've done that this time. But I'm not subscribed so it will need some time. 
Will change my subscription later...
> * runners can go into kdebase, kdeplasma-addons, extragear or with the
> application they relate to; but never kde libs :)
Ok, will think of that next time.
> * i think this should ship with amarok itself; it only makes sense to have
>  it if amarok is there; this could be solved by making it a bit more
>  generic (more on that later) and then it could go into kdeplasma-addons.
I would prefer to make it more generic.

> * the runner is not setting any syntaxes; this prevents runtime help from
> working for this runner. this should be done in reloadConfiguration()
Didn't think of that, sorry. Will be added. 
> * it uses "org.freedesktop.MediaPlayer" so it's actually not amarok
>  specific, except for the start up bits. if the media player was settable
>  (defaulting to amarok) in the configuration, then this wouldn't be an
>  issue. instead of checking for org.kde.amarok to see if it is running, it
>  could just check for org.mpris.<appname> as
>  http://wiki.xmms2.xmms.se/wiki/MPRIS defines it should be
That's something I thought of already. But I wanted Amarok's query method 
(that's the way the runner gets the information about the collection) to be 
used to provide the collection search and this isn't established in the MRIS 
specification. I could use this only if Amarok is selected. Would this be ok?
> * amarokRunning() shouldn't be called in every match(); it should be
>  checked once in a method connected to the prepare() signal, probably
>  setting a bool member that can then be checked for in match().
Will change this, too. 
> * caching the values of songsInPlaylist, prevSongAvailable and
> nextSongAvailable would be a good idea instead of calling them repeatedly
>  in match()
Okay, I will try to do this, too.
> * there is this line in match():
> 
>     searchCollection = config().readEntry("searchCollection", true);
>  //resets searchCollection
> 
> is it really needed?
If I remember correctly it is needed, but I will look into this and try to 
prevent it.

Thanks for your comments to it. Sorry, if I annoyed you with incomplete stuff. 
I don't know how much time I've got to program because my school holidays only 
last one week and I don't know how much time I have if school started again.
Should the runner stay in kdereview or should it be moved to playground again?

Cheers, Jan


More information about the Plasma-devel mailing list