> On April 2, 2013, 5:53 p.m., Kevin Krammer wrote: > > resources/facebook/CMakeLists.txt, line 18 > > <http://git.reviewboard.kde.org/r/109825/diff/1/?file=127841#file127841line18> > > > > the resource's identifier is akonadi_facebook_resource, any reason this > > is _agent?
Not really, I copied it from other resource, which is actually agent. I'll change to _resource. > On April 2, 2013, 5:53 p.m., Kevin Krammer wrote: > > resources/facebook/serializer/akonadi_serializer_socialnotification.cpp, > > line 101 > > <http://git.reviewboard.kde.org/r/109825/diff/1/?file=127845#file127845line101> > > > > this is actually a bugfix, right? > > maybe do this as a separate commit to 4.10 branch and merge to master > > so it is properly independent from the feature commit? Yes, that was my plan :) > On April 2, 2013, 5:53 p.m., Kevin Krammer wrote: > > resources/facebook/facebookresource_notifications.cpp, line 231 > > <http://git.reviewboard.kde.org/r/109825/diff/1/?file=127844#file127844line231> > > > > this also looks like something that would make sense as a plural form > > string This string will never be displayed if .size() == 1, does it still make sense to use i18np()? > On April 2, 2013, 5:53 p.m., Kevin Krammer wrote: > > resources/facebook/facebookresource_notifications.cpp, line 265 > > <http://git.reviewboard.kde.org/r/109825/diff/1/?file=127844#file127844line265> > > > > if you remove an element at i, shouldn't you check i again? > > Yeah, you're right. I'll replace it with iterator. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109825/#review30284 ----------------------------------------------------------- On April 2, 2013, 4:47 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109825/ > ----------------------------------------------------------- > > (Updated April 2, 2013, 4:47 p.m.) > > > Review request for KDEPIM and Plasma. > > > Description > ------- > > This patch displays a KNotification whenever an unread notification exists on > Facebook. This KNotification groups at most 3 notifications into one popup > and then say "...and N more" if there is more. It also keeps track of which > notifications were already displayed and does not display them again unless > they were updated on the server. This is all stored in a separate config > file. > > Then it creates a KSNI for the notifications where it always show the newest > three notifications in the tooltip (regardless if it was displayed before or > not) and creates a menu with the notifications, which opens browser with the > notification link. > > > Diffs > ----- > > resources/facebook/CMakeLists.txt e8c6381 > resources/facebook/facebookresource.h 4a16c0c > resources/facebook/facebookresource.cpp 67e8f3b > resources/facebook/facebookresource_notifications.cpp 7f6b8c4 > resources/facebook/serializer/akonadi_serializer_socialnotification.cpp > a261e14 > resources/facebook/settingsbase.kcfg 9f8e4b5 > resources/facebook/settingsdialog.cpp bfb7826 > resources/facebook/settingsdialog.ui 68b6a24 > > Diff: http://git.reviewboard.kde.org/r/109825/diff/ > > > Testing > ------- > > Yes. > > > File Attachments > ---------------- > > KSNI > > http://git.reviewboard.kde.org/media/uploaded/files/2013/04/02/facebook_notifications.png > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
