Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Sebastian Kügler
> On Feb. 21, 2015, 2:46 a.m., Hrvoje Senjan wrote: > > can you check what needs to be adjusted in plasma-workspace? it fails to > > build with your change: > > > > > > > > [ 451s] > > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43: > > error: 'pr

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Bhushan Shah
> On Feb. 21, 2015, 8:16 a.m., Hrvoje Senjan wrote: > > can you check what needs to be adjusted in plasma-workspace? it fails to > > build with your change: > > > > > > > > [ 451s] > > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43: > > error: 'pr

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi
> On Feb. 21, 2015, 2:46 a.m., Hrvoje Senjan wrote: > > can you check what needs to be adjusted in plasma-workspace? it fails to > > build with your change: > > > > > > > > [ 451s] > > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43: > > error: 'pr

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi
> On Feb. 21, 2015, 2:46 a.m., Hrvoje Senjan wrote: > > can you check what needs to be adjusted in plasma-workspace? it fails to > > build with your change: > > > > > > > > [ 451s] > > /home/abuild/rpmbuild/BUILD/plasma-workspace-5.2.90git/libtaskmanager/taskitem.cpp:471:43: > > error: 'pr

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Hrvoje Senjan
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review76358 --- can you check what needs to be adjusted in plasma-workspace? i

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Feb. 20, 2015, 9:46 p.m.) Status -- This change has been ma

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-21 Thread Gregor Mi
> On Feb. 17, 2015, 10:22 p.m., David Edmundson wrote: > > Thx for looking into it. Since it is quite a large change I'll keep my intermediate commits and push them now. - Gregor --- This is an automatically generated e-mail. To reply,

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-17 Thread David Edmundson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review76201 --- Ship it! - David Edmundson On Feb. 15, 2015, 4:35 p.m., Gre

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-15 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Feb. 15, 2015, 4:35 p.m.) Review request for KDE Base Apps, Domi

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-02-15 Thread Gregor Mi
> On Jan. 21, 2015, 10:10 a.m., Dominik Haumann wrote: > > processcore/processes_linux_p.cpp, line 171 > > > > > > Don't you change behavior here? > > > > Before, we just wrote into the varialbes. > >

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-30 Thread Gregor Mi
> On Jan. 21, 2015, 10:10 a.m., Dominik Haumann wrote: > > processcore/processes_linux_p.cpp, line 171 > > > > > > Don't you change behavior here? > > > > Before, we just wrote into the varialbes. > >

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-30 Thread Gregor Mi
> On Jan. 21, 2015, 10:10 a.m., Dominik Haumann wrote: > > processcore/processes_linux_p.cpp, line 159 > > > > > > In theorey, if we wanted to avoid the local varialbes here, we could > > add reference-getters,

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Gregor Mi
> On Jan. 21, 2015, 10:10 a.m., Dominik Haumann wrote: > > processcore/processes_linux_p.cpp, line 159 > > > > > > In theorey, if we wanted to avoid the local varialbes here, we could > > add reference-getters,

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Thomas Lübking
> On Jan. 21, 2015, 10:10 vorm., Dominik Haumann wrote: > > processcore/process.h, line 40 > > > > > > Is virtual needed here? > > Gregor Mi wrote: > Does it hurt here to have it virtual? I thought, if in dou

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Thomas Lübking
> On Jan. 21, 2015, 10:10 vorm., Dominik Haumann wrote: > > processcore/process.h, line 40 > > > > > > Is virtual needed here? > > Gregor Mi wrote: > Does it hurt here to have it virtual? I thought, if in dou

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Dominik Haumann
> On Jan. 21, 2015, 10:10 vorm., Dominik Haumann wrote: > > processcore/process.h, line 40 > > > > > > Is virtual needed here? > > Gregor Mi wrote: > Does it hurt here to have it virtual? I thought, if in dou

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Thomas Lübking
> On Jan. 21, 2015, 10:10 vorm., Dominik Haumann wrote: > > processcore/process.h, line 40 > > > > > > Is virtual needed here? > > Gregor Mi wrote: > Does it hurt here to have it virtual? I thought, if in dou

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Dominik Haumann
> On Jan. 21, 2015, 10:10 vorm., Dominik Haumann wrote: > > processcore/process.h, line 40 > > > > > > Is virtual needed here? > > Gregor Mi wrote: > Does it hurt here to have it virtual? I thought, if in dou

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 25, 2015, 10:27 p.m.) Review request for KDE Base Apps, Dom

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 25, 2015, 10:20 p.m.) Review request for KDE Base Apps, Dom

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 25, 2015, 10:08 p.m.) Review request for KDE Base Apps, Dom

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Gregor Mi
> On Jan. 21, 2015, 10:10 a.m., Dominik Haumann wrote: > > processcore/process.h, line 40 > > > > > > Is virtual needed here? Does it hurt here to have it virtual? I thought, if in doubt 'virtual' should be used

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-26 Thread Gregor Mi
> On Jan. 25, 2015, 5:28 p.m., Alex Richardson wrote: > > processui/scripting.h, line 74 > > > > > > Isn't just changing the PROPERTY macro enough? > > Or is it used in some other file? > > Gregor Mi wrote: >

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-25 Thread Gregor Mi
> On Jan. 25, 2015, 5:28 p.m., Alex Richardson wrote: > > processui/scripting.h, line 74 > > > > > > Isn't just changing the PROPERTY macro enough? > > Or is it used in some other file? I did the refactoring

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-25 Thread Alex Richardson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review74713 --- Other than these two issues looks good to me, but someone else

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-25 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 25, 2015, 12:01 p.m.) Review request for KDE Base Apps, Dom

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-25 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 25, 2015, 12:46 a.m.) Review request for KDE Base Apps, Dom

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-21 Thread Dominik Haumann
> On Jan. 19, 2015, 6:24 nachm., Aleix Pol Gonzalez wrote: > > processcore/process.h, line 103 > > > > > > Maybe you want to prefix the attribute variables with m_ then? > > Gregor Mi wrote: > Thanks for the

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-21 Thread Dominik Haumann
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review74463 --- I think it's a good idea to hide the impl behind a d-pointer.

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-19 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 19, 2015, 9:37 p.m.) Review request for KDE Base Apps, Domi

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-19 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 19, 2015, 9:33 p.m.) Review request for KDE Base Apps and J

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-19 Thread Gregor Mi
> On Jan. 19, 2015, 6:24 p.m., Aleix Pol Gonzalez wrote: > > processcore/process.h, line 103 > > > > > > Maybe you want to prefix the attribute variables with m_ then? Thanks for the hint. I will move this membe

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-19 Thread Aleix Pol Gonzalez
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review74353 --- processcore/process.h

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-19 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 19, 2015, 6:17 p.m.) Review request for KDE Base Apps, Domi

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-18 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 18, 2015, 11:03 p.m.) Review request for KDE Base Apps, Eik

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-15 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review74100 --- see also last comments in https://git.reviewboard.kde.org/r/12

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-15 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/#review74096 --- processcore/process.h

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-12 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 12, 2015, 2:30 p.m.) Review request for KDE Base Apps and J

Re: Review Request 121831: libksysguard: process.h: encapsulate private fields

2015-01-12 Thread Gregor Mi
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121831/ --- (Updated Jan. 12, 2015, 2:24 p.m.) Review request for KDE Base Apps and J