[Kde-graphics-devel] Review Request 122737: Re-origanise code in the frameworks branch

[Aaron J. Seigo]

> On Feb. 27, 2015, 5:09 p.m., Aaron J. Seigo wrote:
> > Too many things in one patch.
> > 
> > Until the frameworks branch is merged into master, I don't want files moving around unless there is a *compelling* reason for it, and in this case there isn't.
> > 
> > Moving the version definition to the cmake file is fine; adding guards to the generated header is also fine.
> 
> Boudhayan Gupta wrote:
>     Thanks for the feedback.
>     
>     I can revert the moves, and submit a new patch with just moving version definition to CMakeLists and guarding the headers. About the re-factored CMakeLists - should I make only minimal changes (just adding the version definitions), or tidy it up so that if one day the files move around, creating the new CMakeLists for the subdirs is just a matter of cutting out bits from the top level file and pasting them into the subdir ones?

tidying it up is cool, and probably much needed. thanks (in advance) for reworking the patch


- Aaron J.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122737/#review76733
-----------------------------------------------------------


On Feb. 27, 2015, 7:06 p.m., Boudhayan Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122737/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2015, 7:06 p.m.)
> 
> 
> Review request for KSnapshot and Aleix Pol Gonzalez.
> 
> 
> Repository: ksnapshot
> 
> 
> Description
> -------
> 
> This is a pretty big patch. It does the following:
> 
> * Move all the .cpp and .h files to an "src" directory
> * Move the XDG desktop file to a "desktop" directory
> * Move the SVG icon source for the hicolor icon to the icons directory
> * Refactor the CMakeLists.txt file
> * Give each subdirectory (src, icons, desktop) its own CMakeLists.txt. doc already had one.
> * Guard config-ksnapshot.h.cmake contents within ifndef-define-endif preprocessor directives
> * Move the KSNAPVERSION define from main.cpp to config-ksnapshot.h.cmake
> * The KSnapshot version is now defined in the top-level CMakeLists.txt
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt d174abf 
>   Messages-i18n.sh PRE-CREATION 
>   Messages-qt.sh PRE-CREATION 
>   Messages.sh d242e38 
>   config-ksnapshot.h.cmake 3514dd5 
>   main.cpp ef03554 
> 
> Diff: https://git.reviewboard.kde.org/r/122737/diff/
> 
> 
> Testing
> -------
> 
> Builds and installs fine. No functionality was added in this patch, so didn't test much beyond build/run/take screenshots.
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-graphics-devel/attachments/20150227/08545445/attachment.html>