2010/5/27 Christophe Olinger <oling...@binarylooks.com> > Wow, thanks. My 4th accepted patch. Yay! I'll try to commit this > evening or latest tomorrow. > > Concerning the bugs, I can collect them on our techbase page for now. >
Yeah, great idea! So we give them more visibility among eventual other contributors. I'd likely try to figure out which of them can represent junior jobs for new contributors :-) > > Cheers, > Ciao :-) > > Christophe > > > > > On Wed, May 26, 2010 at 11:15 PM, Alessandro Diaferia > <alediafe...@gmail.com> wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviewboard.kde.org/r/4152/#review5876 > > ----------------------------------------------------------- > > > > Ship it! > > > > > > This is a good patch: the move to new animator API is a really good thing > and i wanted this to be done as soon as possible. I still didn't have a look > at the video widget size bug as i still don't have videos on my new > installation. Anyway i'd say go for this patch and i'll try to fix the bug > in the next patch i'm working on. > > > > Also i'd like you to take note of such bugs you encounter as we don't > have a component on bko still (i probably should ask for one). > > > > Here follows little issues i noticed. > > > > > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp > > <http://reviewboard.kde.org/r/4152/#comment5502> > > > > please add a little comment and specify we're returning to browsing > mode as soon as slide-show is finished > > > > > > > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp > > <http://reviewboard.kde.org/r/4152/#comment5503> > > > > please prefer member accessors to array-type ones: > m_pictureMedias.first(); > > > > > > > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp > > <http://reviewboard.kde.org/r/4152/#comment5504> > > > > you can put this as a static const qreal MARGINFACTOR = 0.2; at the > beginning of the code > > > > > > > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp > > <http://reviewboard.kde.org/r/4152/#comment5505> > > > > this line doesn't do anything? you probably want to do > m_controlAutohide = !set; > > > > > > - Alessandro > > > > > > On 2010-05-26 08:19:03, Christophe Olinger wrote: > >> > >> ----------------------------------------------------------- > >> This is an automatically generated e-mail. To reply, visit: > >> http://reviewboard.kde.org/r/4152/ > >> ----------------------------------------------------------- > >> > >> (Updated 2010-05-26 08:19:03) > >> > >> > >> Review request for Plasma and Alessandro Diaferia. > >> > >> > >> Summary > >> ------- > >> > >> This patch covers the following: > >> > >> Features: > >> - Use new plasma animation classes > >> - In floating mode, fade player between pictures (avoids painting > artefacts) > >> - Return to browser after showing last picture of a slideshow > >> - Start at first picture again after slideshow has been played and is > started again > >> - Background states are handled onExit of each state by checking the > currentPlayback state > >> - Renamed layoutZone enum members > >> > >> Bugfixes: > >> - Fixed bug where video player would be fully expanded horizontally, but > not vertically > >> - Slideshow now works correctly > >> - Bottom Panel should be correctly placed now at startup > >> > >> Evil bugs: > >> - Video player always remains at 1024 x 600 :-/. Alessandro, could you > have a look please? > >> - Qt 4.7 beta 1 does not solve the blue skin bug when playing videos > >> - Layout errors of bottom bar (will fix that in the next days) > >> > >> > >> Diffs > >> ----- > >> > >> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.h > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/private/mediahandler.cpp > 1130736 > >> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp > 1130736 > >> > >> Diff: http://reviewboard.kde.org/r/4152/diff > >> > >> > >> Testing > >> ------- > >> > >> > >> Thanks, > >> > >> Christophe > >> > >> > > > > > -- Alessandro Diaferia KDE Developer KDE e.V. member
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
