D10972: [RFC] Exposing slideshow to MPRIS controllers

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Thu Mar 15 00:51:31 UTC 2018


kossebau added a comment.


  In D10972#226088 <https://phabricator.kde.org/D10972#226088>, @rkflx wrote:
  
  > Thanks so much, Friedrich. Latest changes work great, only one more small thing found (but solvable, I guess).
  >
  > I know I have been asking for a lot in this review, but I think users will appreciate the polish. After editing the commit message, let's land this ;)
  
  
  I also appreciated your feedback (incl. the extra effort to get this done in time for freeze), as it gave me the feeling this is a welcome feature and at least for now seems useful also to you :)
  
  Will edit the commit message into something, do you want to review the text first? Would text follwing the one of D11250 <https://phabricator.kde.org/D11250> work for you?
  
  > There's now also D10972: [RFC] Exposing slideshow to MPRIS controllers <https://phabricator.kde.org/D10972> where I added some of the things which could be solved later, so they are not forgotten and interested contributors can jump right in. Feel free to add to this (but please keep it concise) or add subtasks.
  
  Okay. Might make sense to copy the phab T numbers then in the respective code comments, so people can find things both ways?

INLINE COMMENTS

> rkflx wrote in mprismediaplayer2.cpp:82
> This is the only part which prevents compiling with Qt 5.6 as found on Leap 42.3 and elsewhere, which would be kinda nice.
> 
> Are there any alternatives? Or could we return `QString()` for older Qts? The basic functionality still seemed fine to me when testing, so perhaps this is not strictly needed?
> 
> Otherwise we'd have to bite the bullet and bump the Qt version to 5.7.

Ah, missed the Qt 5.6 min dep, right.

https://specifications.freedesktop.org/mpris-spec/latest/Media_Player.html#Property:DesktopEntry says that "[t]This property is optional.".

So I would  simply "if version > 5.6" this part, okay?

REPOSITORY
  R260 Gwenview

BRANCH
  addmprisservice

REVISION DETAIL
  https://phabricator.kde.org/D10972

To: kossebau, #gwenview, rkflx
Cc: mtijink, ngraham, nicolasfella, #kde_connect, rkflx, broulik
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180315/f750288a/attachment-0001.html>


More information about the KDEConnect mailing list