Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-21 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 21, 2015, 6:39 p.m.) Status -- This change has been ma

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-16 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 16, 2015, 3:07 p.m.) Review request for KDE Frameworks and

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-16 Thread Martin Klapetek
> On July 13, 2015, 8:44 p.m., Sune Vuorela wrote: > > src/notifybypopup.cpp, line 565 > > > > > > Isn,t QFont() the same as QApplication::font(), and then the #include > > seems unused. It is the same but Q(G

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-13 Thread Sune Vuorela
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82467 --- In general looks good. src/knotificationmanager.cpp (line 29

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-09 Thread Martin Klapetek
> On July 9, 2015, 4 p.m., Kai Uwe Broulik wrote: > > src/knotificationmanager.cpp, lines 28-29 > > > > > > Unused Fixed locally, thanks. - Martin --- T

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-09 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82269 --- src/knotificationmanager.cpp (lines 28 - 29)

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-09 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 9, 2015, 3:10 p.m.) Review request for KDE Frameworks and M

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-08 Thread Alex Richardson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82234 --- src/knotificationmanager.cpp (line 92)

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-08 Thread Mark Gaiser
On jul 7, 2015, 7:38 p.m., Martin Klapetek wrote: > > I'm looking at the KNotifications dependency graph here [1] and see that > > KWindowSystem is only required for KCrash. > > So err, can't that one go as well since you removed KService which required > > KCrash which then required KWindowSys

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Klapetek
> On July 7, 2015, 9:38 p.m., Mark Gaiser wrote: > > src/knotificationmanager.cpp, line 89 > > > > > > Don't you have a memory leak here? > > You have a list of pointers which you own here, but QList doesn't k

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Mark Gaiser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82195 --- src/knotificationmanager.cpp (line 87)

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 7, 2015, 9 p.m.) Review request for KDE Frameworks and Mart

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Klapetek
> On July 7, 2015, 5:10 p.m., Alex Richardson wrote: > > src/knotificationmanager.cpp, line 89 > > > > > > Use `KPluginLoader::findPlugins` or `KPluginLoader::instantiatePlugins` > > and then do the qobject_cast

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Alex Richardson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82179 --- Looks good to me otherwise src/knotificationmanager.cpp (lin

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 7, 2015, 4:42 p.m.) Review request for KDE Frameworks and M

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Klapetek
> On July 7, 2015, 3:56 p.m., Aleix Pol Gonzalez wrote: > > src/knotificationmanager.cpp, line 89 > > > > > > Would it make any sense to remove this altogether? > > > > If nobody is using it, it sounds us

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82175 --- I like it! - Martin Gräßlin On July 7, 2015, 2:52 p.m., Mar

Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82172 --- src/knotificationmanager.cpp (line 87)

Review Request 124281: Remove KService and KIconThemes usage from KNotifications

2015-07-07 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- Review request for KDE Frameworks and Martin Gräßlin. Repository: knotifi