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

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 https://git.reviewboard.kde.org/r/124281/diff/4/?file=383900#file383900line565 Isn,t QFont() the same as QApplication::font(), and then the #include QApplication seems unused. It is the same but

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

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 https://git.reviewboard.kde.org/r/124281/diff/4/?file=383898#file383898line28 Unused Fixed locally, thanks. - Martin --- This is an

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

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 KWindowSystem?

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
On July 7, 2015, 5:10 p.m., Alex Richardson wrote: src/knotificationmanager.cpp, line 89 https://git.reviewboard.kde.org/r/124281/diff/2/?file=383580#file383580line89 Use `KPluginLoader::findPlugins` or `KPluginLoader::instantiatePlugins` and then do the qobject_cast to avoid

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

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 https://git.reviewboard.kde.org/r/124281/diff/3/?file=383605#file383605line89 Don't you have a memory leak here? You have a list of pointers which you own here, but QList doesn't know that.

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

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:

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.,

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)

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 https://git.reviewboard.kde.org/r/124281/diff/1/?file=383552#file383552line89 Would it make any sense to remove this altogether? If nobody is using it, it sounds useless.

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