Re: Review Request: Add next and previous buttons to Frame applet
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/#review1833 --- Ship it! Patch looks fine, other than some minor coding style things, it's good to go IMO trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp http://reviewboard.kde.org/r/1028/#comment1199 we use braces also for single indented lines trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp http://reviewboard.kde.org/r/1028/#comment1200 braces please also for single lines - Sebastian On 2009-07-28 20:22:17, Arthur Mello wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/ --- (Updated 2009-07-28 20:22:17) Review request for Plasma. Summary --- As mentioned on Frame TODO this patch adds buttons to navigate through slide show. Buttons appear when mouse is over applet and only when applet is doing a slideshow. Example code at TODO put the buttons above the pictue, I placed them on left and right borders, but I can change this if necessary. Diffs - trunk/KDE/kdeplasma-addons/applets/frame/frame.h 1003781 trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 1003781 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 1003781 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 1003781 Diff: http://reviewboard.kde.org/r/1028/diff Testing --- Screenshots --- Slide show buttons http://reviewboard.kde.org/r/1028/s/155/ Thanks, Arthur ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add next and previous buttons to Frame applet
On Friday 17 July 2009 23:06:56 Sebastian Kügler wrote: On Friday 17 July 2009 18:25:06 Arthur Renato Mello wrote: On Fri, Jul 17, 2009 at 9:10 AM, Sebastian Küglerse...@kde.org wrote: However, I have rather substantial changes to the frame applet on my disk. I'm resolving some issues that I wouldn't like to see committed this weekend and am planning to commit the whole thing this weekend. It would be good if I didn't have to rebase all my patches -- I had to do it by hand once already. So please wait with committing it until my work is in. (Your patch looks much easier to rebase on top of mine.) ok. I've committed the changes to the picture frame applet now as r999011. I was working on play/pause too. After you commit I rediff, with pause button, and update the review request. -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9 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
Re: Review Request: Add next and previous buttons to Frame applet
On Tuesday 14 July 2009 19:36:08 Arthur Mello wrote: As mentioned on Frame TODO this patch adds buttons to navigate through slide show. Buttons appear when mouse is over applet and only when applet is doing a slideshow. Example code at TODO put the buttons above the pictue, I placed them on left and right borders, but I can change this if necessary. The approach looks sensible, so +1 for committing this patch. However, I have rather substantial changes to the frame applet on my disk. I'm resolving some issues that I wouldn't like to see committed this weekend and am planning to commit the whole thing this weekend. It would be good if I didn't have to rebase all my patches -- I had to do it by hand once already. So please wait with committing it until my work is in. (Your patch looks much easier to rebase on top of mine.) -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9 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
Re: Review Request: Add next and previous buttons to Frame applet
On Friday 17 July 2009 18:25:06 Arthur Renato Mello wrote: On Fri, Jul 17, 2009 at 9:10 AM, Sebastian Küglerse...@kde.org wrote: However, I have rather substantial changes to the frame applet on my disk. I'm resolving some issues that I wouldn't like to see committed this weekend and am planning to commit the whole thing this weekend. It would be good if I didn't have to rebase all my patches -- I had to do it by hand once already. So please wait with committing it until my work is in. (Your patch looks much easier to rebase on top of mine.) ok. I was working on play/pause too. After you commit I rediff, with pause button, and update the review request. Phew, thanks :) I wish we had a git setup already to make this kind of stuff much easier... -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9 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
Review Request: Add next and previous buttons to Frame applet
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/ --- Review request for Plasma. Summary --- As mentioned on Frame TODO this patch adds buttons to navigate through slide show. Buttons appear when mouse is over applet and only when applet is doing a slideshow. Example code at TODO put the buttons above the pictue, I placed them on left and right borders, but I can change this if necessary. Diffs - trunk/KDE/kdeplasma-addons/applets/frame/frame.h 996678 trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 996678 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 996678 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 996678 Diff: http://reviewboard.kde.org/r/1028/diff Testing --- Screenshots --- SlideShow buttons http://reviewboard.kde.org/r/1028/s/142/ Thanks, Arthur ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add next and previous buttons to Frame applet
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/#review1601 --- Works well and I don't see anything wrong in code on a quick look. Sebas, can you take a quick look as you'll add remote URL support? Thanks Arthur for this patch! A pause button was also in the wish list... ;) - Anne-Marie On 2009-07-14 17:36:08, Arthur Mello wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/ --- (Updated 2009-07-14 17:36:08) Review request for Plasma. Summary --- As mentioned on Frame TODO this patch adds buttons to navigate through slide show. Buttons appear when mouse is over applet and only when applet is doing a slideshow. Example code at TODO put the buttons above the pictue, I placed them on left and right borders, but I can change this if necessary. Diffs - trunk/KDE/kdeplasma-addons/applets/frame/frame.h 996678 trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 996678 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 996678 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 996678 Diff: http://reviewboard.kde.org/r/1028/diff Testing --- Screenshots --- SlideShow buttons http://reviewboard.kde.org/r/1028/s/142/ Thanks, Arthur ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add next and previous buttons to Frame applet
On 2009-07-14 21:04:59, Anne-Marie Mahfouf wrote: Works well and I don't see anything wrong in code on a quick look. Sebas, can you take a quick look as you'll add remote URL support? Thanks Arthur for this patch! A pause button was also in the wish list... ;) Referring to https://bugs.kde.org/show_bug.cgi?id=165704 - Anne-Marie --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/#review1601 --- On 2009-07-14 17:36:08, Arthur Mello wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1028/ --- (Updated 2009-07-14 17:36:08) Review request for Plasma. Summary --- As mentioned on Frame TODO this patch adds buttons to navigate through slide show. Buttons appear when mouse is over applet and only when applet is doing a slideshow. Example code at TODO put the buttons above the pictue, I placed them on left and right borders, but I can change this if necessary. Diffs - trunk/KDE/kdeplasma-addons/applets/frame/frame.h 996678 trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp 996678 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.h 996678 trunk/KDE/kdeplasma-addons/applets/frame/slideshow.cpp 996678 Diff: http://reviewboard.kde.org/r/1028/diff Testing --- Screenshots --- SlideShow buttons http://reviewboard.kde.org/r/1028/s/142/ Thanks, Arthur ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
