Re: Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Maxim Mikityanskiy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124253/ --- (Updated July 4, 2015, 4:36 p.m.) Status -- This change has been mar

Re: Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Maxim Mikityanskiy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124253/#review82078 --- Ship it! Ship It! - Maxim Mikityanskiy On Июль 4, 2015, 4:

Re: Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Maxim Mikityanskiy
> On Июль 4, 2015, 4:18 п.п., Kai Uwe Broulik wrote: > > daemon/backends/upower/login1suspendjob.cpp, line 83 > > > > > > If you're at it, please change this to the new connect syntax: > > > > connect(wat

Re: Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Maxim Mikityanskiy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124253/ --- (Updated Июль 4, 2015, 4:32 п.п.) Review request for kde-workspace. Cha

Re: Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Kai Uwe Broulik
> On Juli 4, 2015, 4:18 nachm., Kai Uwe Broulik wrote: > > daemon/backends/upower/login1suspendjob.cpp, line 83 > > > > > > If you're at it, please change this to the new connect syntax: > > > > connect(w

Re: Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Kai Uwe Broulik
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124253/#review82073 --- Ship it! Thanks! daemon/backends/upower/login1suspendjob.cp

Review Request 124253: Resurrect dead code and fix two memory leaks

2015-07-04 Thread Maxim Mikityanskiy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124253/ --- Review request for kde-workspace. Repository: powerdevil Description --

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
Hi Burkhard, > Some minor nitpicking: > Docbook: > Afaik Alt+Space is default kf5 shortcut for KRunner, wrong in docbook > Shortcut Ctrl+C and Esc missing in docbook > Resizable Window, where resizing the window ends up resizing the screenshot in > it missing in docbook The docbook is very incons

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
Hi Alexander, >> No, some of them stay over in the header file. "Everything in one >> place", for a small application such as this, which is not a public >> library, takes precedence over minorly increased build times. > > Again, if you follow the links [1,2], then Foo.cpp should have > #include

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Alexander Potashev
Boudhayan, Please find my comment below. 2015-07-04 14:29 GMT+03:00 Boudhayan Gupta : >> You said "I can see exactly which components a single cpp file depends >> on my looking at the headers". If you follow the recommendation in the >> links [1,2], all the dependent components will be included i

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
Hi Alexander, > It doesn't seem right to apply different rules to headers that are > exported and not exported. Are you going to rewrite the includes > if/when the class ImageGrabber becomes a public library? (E.g. when > someone wants to incorporate a screen grabber into his application.) Nothin

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Alexander Potashev
2015-07-04 13:11 GMT+03:00 Boudhayan Gupta : >> [1] http://programmers.stackexchange.com/a/262020 >> [2] http://stackoverflow.com/a/15420950 > > I'd seen both of these topics before. I'd obviously use minimal > headers in libraries and other projects where the headers could be > used by the public

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
Hi Alexander, > Projects use different coding styles because each has its pros and > cons. But regarding this #include thing the solution recommended in > the links below is much more advantageous than what you decided to do. > > [1] http://programmers.stackexchange.com/a/262020 > [2] http://stack

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Alexander Potashev
2015-07-04 12:43 GMT+03:00 Boudhayan Gupta : > You've gone ahead and moved all the includes from the header file to > the .cpp file? I'm going to revert that commit, since the decision to > keep all the #includes in the header is a coding style decision I > consciously made. I know this extends bui

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
On 4 July 2015 at 15:15, Burkhard Lück wrote: > Am Samstag, 4. Juli 2015, 15:00:48 schrieb Boudhayan Gupta: >> This has been fixed in the latest commit I did last night. There's >> also a new mode which allows you to shoot Transients along with the >> parent window in the same image. > > Is this a

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Burkhard Lück
Am Samstag, 4. Juli 2015, 15:00:48 schrieb Boudhayan Gupta: > This has been fixed in the latest commit I did last night. There's > also a new mode which allows you to shoot Transients along with the > parent window in the same image. Is this already mentioned in the docbook? -- Burkhard Lück

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
On 4 July 2015 at 05:30, Alexander Potashev wrote: > 2015-07-04 0:53 GMT+03:00 Albert Astals Cid : >> For some reason /usr/include/x86_64-linux-gnu/qt5/QtX11Extras is not in the >> include path. >> >> Any idea why? > > Fixed in > http://commits.kde.org/kscreengenie/ace898e614d612a84ee12f22562d6d

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Boudhayan Gupta
On 4 July 2015 at 14:56, Thomas Lübking wrote: > "Window under mouse" actually shoots the active window what makes it > ipossible to shoot eg. floating docks (*unrelated* to hiding inactive > utility windows) or mainwindows with a modal transient (ie. kwrite while > there's a fileopen dialog) or w

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Thomas Lübking
On Donnerstag, 2. Juli 2015 18:17:16 CEST, Boudhayan Gupta wrote: Hi, I've fixed up the UI as per the above mail thread, and pushed to master. Download and test! "Window under mouse" actually shoots the active window what makes it ipossible to shoot eg. floating docks (*unrelated* to hiding i

Re: KScreenGenie moved to KDE Review

2015-07-04 Thread Burkhard Lück
Am Donnerstag, 2. Juli 2015, 21:47:16 schrieb Boudhayan Gupta: > Hi, > > I've fixed up the UI as per the above mail thread, and pushed to > master. Download and test! > > Copy to clipboard has UI feedback now with a KMessageBox, > > On 29 June 2015 at 19:51, Boudhayan Gupta wrote: > > Hi Thomas