Moved runner "Amarok" to kdereview

[Aaron J. Seigo]

hi Jan ...

On October 16, 2009, Jan Gerrit Marker wrote:
> I have moved a runner called "Amarok" into kdereview. It is in
> plasma/runners/amarok. I added it to the CMakeLists.txt file, too. I want
>  it to go into the KDE libs.

first, the administrative details (i know, "booooooring" ;):

* you need to send an announcement to kde-core-devel at kde.org as well

* runners can go into kdebase, kdeplasma-addons, extragear or with the 
application they relate to; but never kde libs :)

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

now the technical stuff (less boring ;):

* the runner is not setting any syntaxes; this prevents runtime help from 
working for this runner. this should be done in reloadConfiguration()

* 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

* 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().

* caching the values of songsInPlaylist, prevSongAvailable and 
nextSongAvailable would be a good idea instead of calling them repeatedly in 
match()

* there is this line in match():

    searchCollection = config().readEntry("searchCollection", true); //resets 
searchCollection

is it really needed? 

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20091016/b5406290/attachment.sig