mpris2 engine and nowplaying QML applet

[David Edmundson]

On Thu, May 10, 2012 at 11:09 AM, Alex Merry <kde at randomguy3.me.uk> wrote:
> On 05/05/12 19:00, David Edmundson wrote:
>>
>> Has anyone reviewed the Mpris-dataengine? If not I'll do that this
>> evening.
>
>
> Are you still intending to do this?  I was originally planning to commit the
> dataengine and applet tomorrow, and it certainly needs to be done by next
> Thursday (hard feature freeze).
>
Sorry, I made a start, but then got caught up with work.

Only thing that was important was:

void MultiplexedService::activePlayerChanged(PlayerContainer *container)
{
-    delete m_control.data();
+    m_control.data()->deleteLater()


Other considerations:
mpris2engine.cpp:
serviceName.startsWith("org.mpris.MediaPlayer2.")

you have a #define for "org.mpris.MediaPlayer2" in other places, why
not here too.
(in fact given it's in (now) 3 .cpp files, I would consider a header
file for these constants.)

QString sourceName = serviceName.mid(23);
avoid "magic numbers" at a minimum put a comment at the end of this
line: "length of 'org.mpris2.MediaPlayer2' ".  or even
strlen(MPRIS2_IFACE). Compiler would optimise it out to just put "23"
anyway, but it's easier to see what it's doing.

Ship it!

>
> Alex
>
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel at kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel