Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-07 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/ --- (Updated Jan. 7, 2015, 1:43 p.m.) Status -- This change has been mar

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-07 Thread Martin Klapetek
> On Jan. 7, 2015, 11:13 a.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.h, line 74 > > > > > > Just to check, no-one else will ever delete these windows right? Nope, only the de

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-07 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/#review73345 --- Ship it! applets/notifications/plugin/notificationshelper.h

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-06 Thread Martin Gräßlin
> On Jan. 5, 2015, 4:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-06 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/#review73307 --- The beta is in two days, can we please get this in? - Martin

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-06 Thread Martin Klapetek
> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 51 > > > > > > Use QScopedPointer > > Martin Klapetek wrote: > What's wrong with properly us

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-06 Thread Àlex Fiestas
> On gen. 3, 2015, 8:08 p.m., Kai Uwe Broulik wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 51 > > > > > > Use QScopedPointer > > Martin Klapetek wrote: > What's wrong with properly us

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-06 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/ --- (Updated Jan. 6, 2015, 2:32 p.m.) Review request for Plasma and Kai Uwe B

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread David Edmundson
> On Jan. 5, 2015, 3:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Klapetek
> On Jan. 5, 2015, 4:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Gräßlin
> On Jan. 5, 2015, 4:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Klapetek
> On Jan. 5, 2015, 4:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread David Edmundson
> On Jan. 5, 2015, 3:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Klapetek
> On Jan. 5, 2015, 4:19 p.m., David Edmundson wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 111 > > > > > > you're liable to break here. > > > > sometimes you call this directly n

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Gräßlin
> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 51 > > > > > > Use QScopedPointer > > Martin Klapetek wrote: > What's wrong with properly us

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/#review73170 --- applets/notifications/plugin/notificationshelper.cpp

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Klapetek
> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 51 > > > > > > Use QScopedPointer > > Martin Klapetek wrote: > What's wrong with properly us

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread David Edmundson
> On Jan. 3, 2015, 8:08 p.m., Kai Uwe Broulik wrote: > > applets/notifications/plugin/notificationshelper.cpp, line 51 > > > > > > Use QScopedPointer > > Martin Klapetek wrote: > What's wrong with properly us

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/ --- (Updated Jan. 5, 2015, 2:33 p.m.) Review request for Plasma and Kai Uwe B

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-05 Thread Martin Klapetek
> On Jan. 3, 2015, 9:08 p.m., Kai Uwe Broulik wrote: > > Overall the notification appearance is much cleaner, they don't overlap > > anymore and when they're updated they don't just flicker re-appear but fade > > out and fade in again. > > However, the vertical positioning is completely off wit

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-03 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/#review73046 --- Overall the notification appearance is much cleaner, they don'

Re: Review Request 121360: Rework Plasma's notification positioning code

2015-01-02 Thread Martin Klapetek
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/ --- (Updated Jan. 3, 2015, 1:26 a.m.) Review request for Plasma and Kai Uwe B

Re: Review Request 121360: Rework Plasma's notification positioning code

2014-12-15 Thread Emmanuel Pescosta
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/#review72042 --- Maybe a QReadWriteLock instead of a QMutex to make the locking

Re: Review Request 121360: Rework Plasma's notification positioning code

2014-12-15 Thread Martin Klapetek
> On Dec. 5, 2014, 8:19 p.m., Martin Gräßlin wrote: > > I'm a little bit concerned about the mutex usage. I somehow don't see where > > this could be threaded. If it's using threads due to the rendering, then > > it's important that you create it as a QMutex::Recursive mutex as otherwise > > o

Re: Review Request 121360: Rework Plasma's notification positioning code

2014-12-05 Thread Martin Gräßlin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121360/#review71438 --- I'm a little bit concerned about the mutex usage. I somehow do