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-de...@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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
