----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.vidsolbach.de/r/309/#review296 -----------------------------------------------------------
Ship it! Apart from the comments below, it looks good to me. I'm happy for it to go in providing the translators are happy about the extra string (kde-i18n@, I believe is the mailing list you want). You could get around the string freeze by commenting out line 301 with a FIXME: 4.3 comment, and putting toolTip.setSubText(m_artist); instead for now. I would grab aacid on irc and ask him (or email him @kde.org). /trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.cpp <http://reviewboard.vidsolbach.de/r/309/#comment244> What happens if you Feeling Good by Michael Bublé playing, then the next song is Muse's cover of the same track? /trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.cpp <http://reviewboard.vidsolbach.de/r/309/#comment245> How is "@info:tooltip" a useful comment? Translators need to know what on earth "by" means in this context. How about "song artist, displayed below the song title"? - Alex On 2008-12-14 09:09:14, Tony Murray wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.vidsolbach.de/r/309/ > ----------------------------------------------------------- > > (Updated 2008-12-14 09:09:14) > > > Review request for Plasma and Alex Merry. > > > Summary > ------- > > This adds a tooltip when on the panel. It includes artwork and track title > and artist. It does add one string, so we would have to clear that with > translators if we add it now "by %1". > > It does not update the tooltip while it is show, but the timeout is rather > short so it might not be worth the extra cpu cycles. > > QPixmap.scale handles null pixmaps gracefully, so it isn't necessary to have > a check for that right? > > > Diffs > ----- > > /trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.h > /trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.cpp > > Diff: http://reviewboard.vidsolbach.de/r/309/diff > > > Testing > ------- > > On my local svn machine only. Vertical, Horizontal, and Planar all tested. > (It isn't supposed to show for Planar ;) > > > Thanks, > > Tony > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
